Skip to content
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

fix: Fix RPGdie iuse #5793

Merged
merged 1 commit into from
Dec 5, 2024
Merged

Conversation

KheirFerrum
Copy link
Collaborator

@KheirFerrum KheirFerrum commented Dec 5, 2024

Checklist

Required

Purpose of change

  • Fix Dice are consumed on activation #5789 which was a bug exposed by fix: Make rags consume their charges/delete themselves. #5589. iuse actions in iuse.cpp return an int value that indicates charges to be used from an item, but this wasn't properly being used due to a lack of consideration in use_charges().
  • The RPG die iuse returns the roll value for no reason whatsoever, as nothing the iuse does is contingent on that return. It's likely some legacy thing from when each iuse had it's own special return function.

Describe the solution

  • iuse::rpgdie now returns 0 in all cases, preventing charges of rpgdie from being used.

Describe alternatives you've considered

  • Reverting fix: Make rags consume their charges/delete themselves. #5589
    • Would be a dumb idea given that the behaviour it fixes is not expected behaviour.
  • Rework iuse::rpgdie
    • It's on my list, as of current, the die faces and values are totally random, I'll likely convert it into an iuse_actor later on so that you can define individual dice with fixed sides instead of each die being set randomly to being a d4, d8, d20, d50 etc.

Testing

  • Spawn and roll die and confirm they aren't being used up.

Additional context

I shudder to think what other bugs have been uncovered in the process. Most should be fixed with the energy rework, but that's going to break 50 other things I didn't account for I'm sure.

It shouldn't return the roll value as the return indicates charges to use.
@github-actions github-actions bot added the src changes related to source code. label Dec 5, 2024
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@RoyalFox2140 RoyalFox2140 merged commit e5f4696 into cataclysmbnteam:main Dec 5, 2024
12 checks passed
@KheirFerrum KheirFerrum deleted the Fix-RPG-Dice branch December 6, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dice are consumed on activation
3 participants