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

Re-consider class names / structure / functionality #306

Open
GreenImp opened this issue Mar 6, 2025 · 1 comment
Open

Re-consider class names / structure / functionality #306

GreenImp opened this issue Mar 6, 2025 · 1 comment
Milestone

Comments

@GreenImp
Copy link
Collaborator

GreenImp commented Mar 6, 2025

As part of the re-write, we should look at all of the existing classes and see if they're all necessary and what can be improved.
There are several classes that have very similar names, which could be confusing;

  • DiceRoller and DiceRoll
  • RollResults and RollResult
  • etc.

Other things to consider are things like;

  • Currently the dice classes are responsible for rolling as well. It might be good to change this so that they just store the info (sides, qty, modifiers etc.), and have a new "Roll Engine" which does the rolling.
  • The grammar parser currently converts the notation directly to the various objects. It might be worthwhile creating an intermediary AST of plain objects. This could then allow us greater flexibility with passing them into the final output, and hopefully enabling us to do inline / recursive rolling (see Inline rolls #206).

This list is not exhaustive, but is a good starting point.

@GreenImp GreenImp added this to the 6.0.0 milestone Mar 6, 2025
@GreenImp
Copy link
Collaborator Author

GreenImp commented Mar 8, 2025

DiceRoller

  • Renamed class name to RollLog
  • Added new add() method
  • Renamed clearLog() to clear()
  • Made class iterable (using for...of)

Modifier

  • Renamed run() to apply()

RollEngine

  • New class for rolling dice values

ModifierHandler

  • New class for running modifiers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant