add RGSS_VERSION (RGSS3)

cremno commented

2014-08-31 03:52:40 +00:00

(Migrated from github.com)

Even though 3.0.1 is the current version, 3.0.0 is used, because mkxp doesn't
implement the changes like not raising a TypeError for 'Color.new == nil' yet.
3.0.1 (finally) returns false. Other changes don't affect mkxp (I think).

See http://tkool.jp/support/download/rpgvxace/rpgvxace_update (Japanese).

The comparison change and maybe adding RGSS_VERSION for rgssVer == 1 and rgssVer == 2 requires some discussion.

I think mkxp should implement 3.0.1's behavior for rgssVer == 3 as its usage is higher than 3.0.0, because the international VX Ace defaults to RGSS301.dll since its release.

Even though 3.0.1 is the current version, 3.0.0 is used, because mkxp doesn't implement the changes like not raising a TypeError for 'Color.new == nil' yet. 3.0.1 (finally) returns false. Other changes don't affect mkxp (I think). See http://tkool.jp/support/download/rpgvxace/rpgvxace_update (Japanese). --- The comparison change and maybe adding `RGSS_VERSION` for `rgssVer == 1` and `rgssVer == 2` requires some discussion. I think mkxp should implement 3.0.1's behavior for `rgssVer == 3` as its usage is higher than 3.0.0, because the international VX Ace defaults to RGSS301.dll since its release.

Ancurio commented

2014-08-31 08:57:13 +00:00

(Migrated from github.com)

The comparison change and maybe adding RGSS_VERSION for rgssVer == 1 and rgssVer == 2 requires some discussion.

Yeah, I'm a bit worried about scripts that will intentionally test if this constant is defined to probe whether they're running on RGSS3 or earlier (and adjust their behavior accordingly). IMO it would be wiser to not define it pre 3.

> The comparison change and maybe adding RGSS_VERSION for rgssVer == 1 and rgssVer == 2 requires some discussion. Yeah, I'm a bit worried about scripts that will intentionally test if this constant is defined to probe whether they're running on RGSS3 or earlier (and adjust their behavior accordingly). IMO it would be wiser to not define it pre 3.

cremno commented

2014-08-31 10:03:31 +00:00

(Migrated from github.com)

That could be the case. It was just an idea.

Any comments on the comparison issue? If mkxp also fixes it, the version string can be changed to 3.0.1. Or do you need some time to think about supporting not only major versions? There are some older games that use RGSS300.dll, but I think that change wouldn't break any of them.

That could be the case. It was just an idea. Any comments on the comparison issue? If mkxp also fixes it, the version string can be changed to 3.0.1. Or do you need some time to think about supporting not only major versions? There are some older games that use RGSS300.dll, but I think that change wouldn't break any of them.

Ancurio commented

2014-08-31 10:16:31 +00:00

(Migrated from github.com)

No, fixing the comparison issue and changing the string to "3.0.1" is perfectly fine I think. The only behavior that we're changing is to not crash from a raised exception, right? I don't see any way in which a game could rely on a TypeError being raised.

No, fixing the comparison issue and changing the string to "3.0.1" is perfectly fine I think. The only behavior that we're changing is to _not_ crash from a raised exception, right? I don't see any way in which a game could rely on a TypeError being raised.

cremno commented

2014-08-31 10:23:39 +00:00

(Migrated from github.com)

Until 3.0.1 a Color, Tone or Rect instance can only be compared to another instance of the same class, otherwise a TypeError is raised. 3.0.1 correctly returns false instead.

Until 3.0.1 a Color, Tone or Rect instance can only be compared to another instance of the same class, otherwise a `TypeError` is raised. 3.0.1 correctly returns `false` instead.

Ancurio commented

2014-08-31 10:48:19 +00:00

(Migrated from github.com)

Anyway, that can happen in a later commit. Can you wrap the RGSS_VERSION definition in a if (rgssVer == 3)? Then I can pull this.

Anyway, that can happen in a later commit. Can you wrap the `RGSS_VERSION` definition in a `if (rgssVer == 3)`? Then I can pull this.

cremno commented

2014-08-31 10:55:01 +00:00

(Migrated from github.com)

Isn't it sort of already in one? Should I add an assertion, too?

diff --git a/binding-mri/binding-mri.cpp b/binding-mri/binding-mri.cpp
index 12c9e86..efe7ab3 100644
--- a/binding-mri/binding-mri.cpp
+++ b/binding-mri/binding-mri.cpp
@@ -114,7 +114,10 @@ static void mriBindingInit()
                _rb_define_module_function(rb_mKernel, "msgbox",    mriPrint);
                _rb_define_module_function(rb_mKernel, "msgbox_p",  mriP);

-               rb_define_global_const("RGSS_VERSION", rb_str_new_cstr("3.0.0"))
+               if (rgssVer == 3)
+                       rb_define_global_const("RGSS_VERSION", rb_str_new_cstr("3.0.0"))
+               else
+                       assert(!"unreachable");
        }
        else
        {

Isn't it sort of already in one? Should I add an assertion, too? ``` diff diff --git a/binding-mri/binding-mri.cpp b/binding-mri/binding-mri.cpp index 12c9e86..efe7ab3 100644 --- a/binding-mri/binding-mri.cpp +++ b/binding-mri/binding-mri.cpp @@ -114,7 +114,10 @@ static void mriBindingInit() _rb_define_module_function(rb_mKernel, "msgbox", mriPrint); _rb_define_module_function(rb_mKernel, "msgbox_p", mriP); - rb_define_global_const("RGSS_VERSION", rb_str_new_cstr("3.0.0")) + if (rgssVer == 3) + rb_define_global_const("RGSS_VERSION", rb_str_new_cstr("3.0.0")) + else + assert(!"unreachable"); } else { ```

Ancurio commented

2014-08-31 10:57:59 +00:00

(Migrated from github.com)

Oh, excuse me, you're right; with the small hunk context and the discussion about earlier RGSS versions, I somehow didn't realize this. My bad.

add RGSS_VERSION (RGSS3) #61