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

DiceRoll with provided rolls are not being computed by notation with modifiers #230

Open
frankieali opened this issue Aug 11, 2021 · 4 comments
Labels

Comments

@frankieali
Copy link

frankieali commented Aug 11, 2021

It seems supplying rolls to a DiceRoll class only results in those rolls being added together. Modifiers are not being handled.

const data = {
    notation: '4d6dl1',
    rolls: [
        3, 6, 2, 4,
    ],
};
const roll = new DiceRoll(data);
console.log("roll result",roll) // roll total = 15

I would expect the total of this roll (4d6 drop lowest 1) to be 13, but it is 15.

A more complex example using RollResult yields the same result

const data = {
    notation: '4d6dl1',
    rolls: new Results.ResultGroup([
        new Results.RollResults([
            3, 6, 4,
            new Results.RollResult(2,['drop']),
        ]),
    ])
};
const roll = new DiceRoll(data);
console.log("roll result",roll) // roll.total = 15

This second example will almost match a plain new DiceRoll('4d6dl1') except that the lowest die is not dropped from the total.

Am I doing something wrong?

@frankieali frankieali added the bug label Aug 11, 2021
@GreenImp
Copy link
Collaborator

The import process is very simple; it takes what you give it, and assumes that it's correct. It doesn't run any modifiers or alter the data, as the assumption is that the data has already been modified (Generally by exporting the data from an existing DiceRoll object).

So, using your second example, you would also need to modify the value of the RollResult. The DropModifier in particular, sets the RollResult.useInTotal to false.

Note: The modifiers prop doesn't actually do anything functional, it's just so we know what modifiers were applied, so we can style it differently on output.

// passing `false` as the third parameter sets it to be "ignored" when calculating roll totals
new Results.RollResult(2, ['drop'], false);

So, your example would look something like:

const data = {
    notation: '4d6dl1',
    rolls: new Results.ResultGroup([
        new Results.RollResults([
            3, 6, 4,
            new Results.RollResult(2, ['drop'], false),
        ]),
    ])
};
const roll = new DiceRoll(data);
console.log("roll result", roll) // roll.total = 13

Although you may also be able to do something like this, as an array gets cast to a ResultGroup:

const data = {
    notation: '4d6dl1',
    rolls: [
        3,
        6,
        2,
        new Results.RollResult(2, ['drop'], false),
    ],
};

I think that should work for you. Let me know if not.

@GreenImp GreenImp added modifier Dice modifier question and removed bug labels Aug 11, 2021
@frankieali
Copy link
Author

Yes, setting the useInTotal flag produces the proper output. Thanks for the quick reply. I was hoping that modifiers would run, but I can see why they wouldn't when dealing with previously exported data.

I am building a 3D dice roller. I was hoping I could use the parsed results to determine what dice to roll and then send back the 3D roll results with the original notation to be computed. I don't want to handle the computing of notations manually. That's probably out of scope for this library. Perhaps a future feature?

@GreenImp
Copy link
Collaborator

Hm interesting. I can see that being able to pass through "raw" values, with the notation, and having them passed could be good. Maybe as something like rawRolls. I'll leave this ticket open for looking at in the future. Any PRs are always welcome :)

Out of interest, and knowing nothing about the 3D roller your building; would it be feasible to reverse the logic?
Instead of rolling the 3D dice, and trying to feed the data back to the library, could you parse the notation, and get the results. Then feed them to the 3D dice, and forcing them to roll specific values?

@frankieali
Copy link
Author

I did look into forcing the dice to show a specific face, but it looks very unnatural and janky.
I haven't dug into the source code of this project yet. RPG-Dice-Roller's feature set is more broad than others I've tried so far and might be the only one still under active development. I'll take a deeper dive soon.
Thanks for considering rawRolls. I think it would be useful for users who want to generate their numbers outside of RPG-Dice-Roller.

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

No branches or pull requests

2 participants