-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add MRI 1.8 Support #163
base: master
Are you sure you want to change the base?
Add MRI 1.8 Support #163
Conversation
Also fixed compile for MRI 1.9, but this crashes with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the minor formatting, I can't comment too much on the 1.8 since I'm not familiar with the differences to 2.x, but I'd really like to get this merged to start the effort even if not everything is working yet.
Thanks for your work!
binding-mri/binding-mri.cpp
Outdated
|
||
#if RUBY_API_VERSION_MAJOR == 1 | ||
ruby_init(); | ||
//ruby_options(argc, argv); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dead code comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling this somehow segfaults Ruby and I have no idea why. Will remove it.
binding-mri/binding-util.cpp
Outdated
@@ -90,13 +90,17 @@ void raiseRbExc(const Exception &exc) | |||
void | |||
raiseDisposedAccess(VALUE self) | |||
{ | |||
#if RUBY_API_VERSION_MAJOR == 1 | |||
// FIXME: raiseDisposedAccess not implemented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an issue with implementing this in 1.8? Does 1.8 not allow raising exceptions from C?
Edit: Oh it probably has to do with getting the class name from the typed data object, I see.
binding-mri/binding-util.cpp
Outdated
@@ -319,3 +323,73 @@ rb_get_args(int argc, VALUE *argv, const char *format, ...) | |||
|
|||
return argI; | |||
} | |||
|
|||
#if RUBY_API_VERSION_MAJOR == 1 | |||
// Functions providing Ruby 1.8 compatibililty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use /* */ for documentative comments
binding-mri/binding-util.cpp
Outdated
|
||
#if RUBY_API_VERSION_MAJOR == 1 | ||
// Functions providing Ruby 1.8 compatibililty | ||
VALUE rb_sprintf(const char* fmt, ...) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest of the code base puts opening braces on a new line
binding-mri/filesystem-binding.cpp
Outdated
@@ -130,7 +132,6 @@ kernelLoadDataInt(const char *filename, bool rubyExc) | |||
VALUE port = fileIntForPath(filename, rubyExc); | |||
|
|||
VALUE marsh = rb_const_get(rb_cObject, rb_intern("Marshal")); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
random whitespace change?
binding-mri/binding-util.cpp
Outdated
} | ||
|
||
VALUE rb_hash_lookup2(VALUE, VALUE, VALUE) { | ||
return Qnil; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the equivalent 1.8 function for this?
binding-mri/binding-util.cpp
Outdated
@@ -392,4 +392,5 @@ int rb_utf8_encindex(void) { | |||
VALUE rb_hash_lookup2(VALUE, VALUE, VALUE) { | |||
return Qnil; | |||
} | |||
#endif | |||
#endif // RUBY_API_VERSION_MINOR == 8 | |||
#endif // RUBY_API_VERSION_MAJOR == 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing newline at end
@@ -32,7 +32,7 @@ | |||
#include "boost-hash.h" | |||
|
|||
#include <ruby.h> | |||
#if RUBY_API_VERSION_MAJOR > 1 | |||
#ifndef RUBY_LEGACY_VERSION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this defined by a ruby header, or do we define it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I define it. Problem is that checking for 1.8 vs. 1.9 is ugly to write so I introduced a new macro for 1.8 only. You can suggest a better macro name :D
I'm also coding blind because the documentation for MRI 1.8 is basicly non-existent :D Added raiseDisposedAccess (though the reported classname will be quite useless, just some internal class(#12345678). Though the bookkeeping for storing the "emulated typed data classname" somewhere is not worth the afford imho. So the sanity checks will also not work (they always return true) but I guess they bomb a bit later then. Or do you have any ideas to solve this with not too much work? rb_str_catf and rb_hash_lookup2 are implemented. hash_lookup is quite useless, only used in VX Ace codepath. The stacktrace somehow only always contains one element in my tests and I have no idea why o.O |
Rebased my changes but I can't test if I failed with my rebase because I can't get mkxp to compile because PhysFS changed the API >.>. I also applied @carstene1ns PR, but I still get
Any idea? |
My PR is not yet updated with the change @Ancurio requested... |
Thanks. https://gist.github.com/Ghabry/001c7e920b4a62089220866e7ce68a09 compiles for me now and can confirm that the rebase was successful 👍 |
Thank you for fixing my PR! =) |
Unfortunately crashes on startup with "[BUG] object allocation during garbage collection phase"
do you know if this works with ruby 1.8.1 specifically? iirc, the mkxp-z bindings dont |
Not tested. Because it is a minor update I assumed it does not have any breaking changes. Is there any game that does not work with 1.8.7? |
yeah ive found one thats throwing some subclass error or some shit, anyway tried on ruby 1.8.1,
|
Is this the only error? Replace the content of my rb_hash_lookup2 function with Should be fine because it is only used by rgss3 code |
Well no idea then. |
Don't hang if the user closes the game while it's still initializing
This allows compiling of mkxp against Ruby 1.8.7 (tested with p374).
To test that it works simply start a RPG Maker XP game. If you get "Win32Api not found" instead of "Syntax error" it works ;).
This is mostly implemented by providing wrapper functions and macros (mostly copy-pasted from ruby 2.4) in binding-util.h.
Known issues:
ruby_option (like ruby_sysinit) segfaults when called.call removed for nowrb_hash_lookup2 not implemented but this is only in a RGSS3 codepath, so not relevantrb_str_catf not implemented (so no stacktrace printed in error case)raiseDisposedAccess will never raiseThat typed data stuff was a bit tricky to fake because Ruby 1.8 does not have this typed data struct. But is good enough to launch games. Though some sanity checks are missing (see issues above)
To The Moon (Windows version) with 2.4 MRI:
![screenshot_20170512_023350](https://cloud.githubusercontent.com/assets/1331889/25977362/7a2ca61e-36bb-11e7-96db-786f7854b737.png)
With 1.8 MRI:
![screenshot_20170512_023314](https://cloud.githubusercontent.com/assets/1331889/25977364/7a40b924-36bb-11e7-9221-e87105d4e977.png)
With 1.8 MRI and some WinApi emulation (Mouse cursor is stuck):
![screenshot_20170512_023212](https://cloud.githubusercontent.com/assets/1331889/25977363/7a3a9080-36bb-11e7-9942-47a9b7762b4b.png)