Rational errors fix #239

Open
pk-2000 wants to merge 1 commits from pk-2000/test-RUBY_T_RATIONAL into master
pk-2000 commented 2021-08-31 21:11:07 +00:00 (Migrated from github.com)
  • Better debug information on float errors (lines 231 and 251).
  • line 246 fixes ruby rational errors
- Better debug information on float errors (lines 231 and 251). - line 246 fixes ruby rational errors
cremno (Migrated from github.com) reviewed 2021-09-01 15:36:58 +00:00
@ -251,3 +251,4 @@
rb_raise(rb_eTypeError, "Argument %d: Expected fixnum(got 0x%x)", argPos, rb_type(arg));
}
}
cremno (Migrated from github.com) commented 2021-09-01 15:35:58 +00:00

Have you tested this? FIX2INT() only works with fixnums.

Have you tested this? `FIX2INT()` only works with fixnums.
pk-2000 (Migrated from github.com) reviewed 2021-09-01 17:27:51 +00:00
@ -251,3 +251,4 @@
rb_raise(rb_eTypeError, "Argument %d: Expected fixnum(got 0x%x)", argPos, rb_type(arg));
}
}
pk-2000 (Migrated from github.com) commented 2021-09-01 17:27:51 +00:00

Yes,
the only games that I encountered this problem are some rgss1 games,
that use a plugin to determine the characters position.
float value (X,Y position of the character) & rational value (expected X,Y new value)
The new value is determined from the move direction of the character.

Yes, the only games that I encountered this problem are some rgss1 games, that use a plugin to determine the characters position. float value (X,Y position of the character) & rational value (expected X,Y new value) The new value is determined from the move direction of the character.
cremno (Migrated from github.com) reviewed 2021-09-02 15:52:32 +00:00
@ -251,3 +251,4 @@
rb_raise(rb_eTypeError, "Argument %d: Expected fixnum(got 0x%x)", argPos, rb_type(arg));
}
}
cremno (Migrated from github.com) commented 2021-09-02 15:52:32 +00:00

Yeah, RGSS3 also supports rational numbers but your change may only appear to work. Could you please try something like Rect.new(Rational(10, 10), 0, 0, 0) and check the value of its x attribute? I can't compile mkxp at the moment and there's no test suite or build bot.

IMHO the type check isn't necessary anyway. *out = NUM2INT(arg); is good enough (and rb_rescue() if the custom error message should be kept). This would add support for complex numbers too: Rect.new(Complex(10, 0), 0, 0, 0).

Yeah, RGSS3 also supports rational numbers but your change may only appear to work. Could you please try something like `Rect.new(Rational(10, 10), 0, 0, 0)` and check the value of its `x` attribute? I can't compile mkxp at the moment and there's no test suite or build bot. IMHO the type check isn't necessary anyway. `*out = NUM2INT(arg);` is good enough (and `rb_rescue()` if the custom error message should be kept). This would add support for complex numbers too: `Rect.new(Complex(10, 0), 0, 0, 0)`.
pk-2000 (Migrated from github.com) reviewed 2021-09-03 11:58:03 +00:00
@ -251,3 +251,4 @@
rb_raise(rb_eTypeError, "Argument %d: Expected fixnum(got 0x%x)", argPos, rb_type(arg));
}
}
pk-2000 (Migrated from github.com) commented 2021-09-03 11:58:03 +00:00

Can you give another test?
My Falcon-mkxp has unlocked resolution, so the x result will always deffer, depending on the game resolution & monitor resolution & position of the window.

  • I had tested it against these examples
    https://ruby-doc.org/core-2.5.3/Rational.html
    and the only ones that could not resolve (if I remember correctly) were the examples with half keyword arguments e.g.
    Rational(25, 100).round(1, half: :up) #=> (3/10)
Can you give another test? My Falcon-mkxp has unlocked resolution, so the x result will always deffer, depending on the game resolution & monitor resolution & position of the window. - I had tested it against these examples https://ruby-doc.org/core-2.5.3/Rational.html and the only ones that could not resolve (if I remember correctly) were the examples with half keyword arguments e.g. Rational(25, 100).round(1, half: :up) #=> (3/10)
cremno (Migrated from github.com) reviewed 2021-09-04 04:15:12 +00:00
@ -251,3 +251,4 @@
rb_raise(rb_eTypeError, "Argument %d: Expected fixnum(got 0x%x)", argPos, rb_type(arg));
}
}
cremno (Migrated from github.com) commented 2021-09-04 04:15:12 +00:00

X should always be 1. But it can be tested with any other integer attribute:

s = Sprite.new
s.z = Rational(10, 2)
p s.z # => 5
X should always be 1. But it can be tested with any other integer attribute: ```ruby s = Sprite.new s.z = Rational(10, 2) p s.z # => 5 ```
pk-2000 (Migrated from github.com) reviewed 2021-09-04 14:00:51 +00:00
@ -251,3 +251,4 @@
rb_raise(rb_eTypeError, "Argument %d: Expected fixnum(got 0x%x)", argPos, rb_type(arg));
}
}
pk-2000 (Migrated from github.com) commented 2021-09-04 14:00:50 +00:00

Thank you.
I created 2 events to call the test scripts and both of them
return positive random(?) values with 8 digits(!) instead of returning x=1 & s=5.

Thank you. I created 2 events to call the test scripts and both of them return positive random(?) values with 8 digits(!) instead of returning x=1 & s=5.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b pk-2000/test-RUBY_T_RATIONAL master
git pull origin pk-2000/test-RUBY_T_RATIONAL

Step 2:

Merge the changes and update on Forgejo.
git checkout master
git merge --no-ff pk-2000/test-RUBY_T_RATIONAL
git push origin master
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: MapleShrine/mkxp#239
No description provided.