MapleShrine/mkxp
1
0
Fork 0
Code Issues 32 Pull requests 11 Projects Releases Wiki Activity

Rational errors fix #239

Open
pk-2000 wants to merge 1 commit from pk-2000/test-RUBY_T_RATIONAL into master
pull from: pk-2000/test-RUBY_T_RATIONAL
merge into: MapleShrine:master
Conversation 6 Commits 1 Files changed 1 +3 -2
pk-2000 commented 2021-08-31 21:11:07 +00:00 (Migrated from github.com)
Copy link
  • 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
binding-mri/binding-util.h
@ -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
Copy link

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
binding-mri/binding-util.h
@ -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
Copy link

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
binding-mri/binding-util.h
@ -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
Copy link

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
binding-mri/binding-util.h
@ -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
Copy link

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
binding-mri/binding-util.h
@ -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
Copy link

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
binding-mri/binding-util.h
@ -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
Copy link

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.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin pk-2000/test-RUBY_T_RATIONAL:pk-2000/test-RUBY_T_RATIONAL
git checkout pk-2000/test-RUBY_T_RATIONAL

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout master
git merge --no-ff pk-2000/test-RUBY_T_RATIONAL
git checkout pk-2000/test-RUBY_T_RATIONAL
git rebase master
git checkout master
git merge --ff-only pk-2000/test-RUBY_T_RATIONAL
git checkout pk-2000/test-RUBY_T_RATIONAL
git rebase master
git checkout master
git merge --no-ff pk-2000/test-RUBY_T_RATIONAL
git checkout master
git merge --squash pk-2000/test-RUBY_T_RATIONAL
git checkout master
git merge --ff-only pk-2000/test-RUBY_T_RATIONAL
git checkout master
git merge pk-2000/test-RUBY_T_RATIONAL
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
RGSS accuracy

   
bug

   
compilation

   
discussion

   
documentation

   
duplicate

   
enhancement

   
invalid

   
performance issue

   
port request

   
question

   
ruby incompatibility

   
wontfix
No labels
RGSS accuracy
bug
compilation
discussion
documentation
duplicate
enhancement
invalid
performance issue
port request
question
ruby incompatibility
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
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.
Delete branch "pk-2000/test-RUBY_T_RATIONAL"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?