Jump to content

No damage Bug - Overflow behavior


MarkZD

Recommended Posts


  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

http://rathena.org/b...sconnect-issue/

Sample of bug:

damage = 3000000;

cardfix = 1000;

damage = damage * cardfix / 1000;

damage = 3000000 * 1000 / 1000;

damage = 3000000000(overflow, now it's negative) / 1000;

Result: No damage, since damages smaller than 1 will return miss.

Here's a patch I made which will make formulas a little safer on systems(compilers) which implements int as int32:

http://upaste.me/24dc1734408c633f

Or we can try using long long, there's other ways to do.

I'd like to hear your opnion.

EDIT:

RIght after posting I though it'd be more elegant to use int64(long long), instead of using the float trick.

Sample:

damage = (int64) damage * cardfix / 1000;

If none has any complain, I'll go for it.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  16
  • Topics Per Day:  0.00
  • Content Count:  737
  • Reputation:   216
  • Joined:  11/29/11
  • Last Seen:  

int64 would be faster yeah and will keep the actual behaviour. But that mean the actual default if ratio is 0 etc.

We probably have some other place where multiplication overflow type limit before whatever division.

About macro I already told you to verify their scope (undef) and I think we should keep the actual ATK_RATE2 as it is but declare the 2 new atk macro as ATK_RATER and ATK_RATEL (since it's to increase left or right dammage). This to not shift the actual one in ATK_RATE3 and increase the diff.

Finally shifting for multiplication don't really matter since makefile have optimisation on. THat would produce the same code as shifting if this si faster, or will shift on is on.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

and I think we should keep the actual ATK_RATE2 as it is but declare the 2 new atk macro as ATK_RATER and ATK_RATEL (since it's to increase left or right dammage). This to not shift the actual one in ATK_RATE3 and increase the diff.

I have already put it, it was a problem when pasting it to uPaste, updated the link.

Edit:

Implemented the safer way in battle.c

r17004

Link to comment
Share on other sites

Guest
This topic is now closed to further replies.
×
×
  • Create New...