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

Allow the use of all Rollables in Dice #85

Open
Samasaur1 opened this issue Jul 13, 2020 · 1 comment
Open

Allow the use of all Rollables in Dice #85

Samasaur1 opened this issue Jul 13, 2020 · 1 comment
Labels
enhancement New feature or request

Comments

@Samasaur1
Copy link
Owner

Is your feature request related to a problem? Please describe.

It doesn't make a lot of sense that WeightedDie cannot be used in Dice, and in a larger sense why any Rollable can't be. Rollable requires implementations of what I'm pretty sure are all the rolling/probabilities methods that Die has that Dice uses. It requires looking into, and refactoring some parts of Dice, but it seems like an overdue feature.

Describe the solution you'd like:

Users should be able to use and Rollable type in Dice. This includes the initializers (probably excluding the dice notation String-based initializer), operators, and pretty much everywhere else.

Describe alternatives you've considered:

Maybe Dice should only allow WeightedDie/Rollable types that I define, but that seems overly restrictive. That's what having a protocol is for.

@Samasaur1 Samasaur1 added enhancement New feature or request needs-labeling An issue that needs to be labeled correctly and removed needs-labeling An issue that needs to be labeled correctly labels Jul 13, 2020
@Samasaur1
Copy link
Owner Author

Especially considering that we convert the dice dictionary to an array before performing calculations anyway:

//Gets an array of all the dice (which can include multiple of the same die) to loop through.
var sortedDice: [Die] = []
for (die, count) in dice {
for _ in 0..<count {
sortedDice.append(die)
}
}
sortedDice = sortedDice.sorted()

and when rolling we need to "expand" the dictionary anyway:

public func roll() -> Roll {
var result = 0
for (die, count) in dice {
for _ in 0..<count {
result += die.roll()
}
}
result += modifier
return result
}

I see no reason why we shouldn't convert the dice dictionary to (presumably) an any Rollable array

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

No branches or pull requests

1 participant