10 sided dice report incorrect "label"

[tim-duncan]

While playing around I noticed that all the 10 sided dice (d10, d100) report the "label" value as off by 1.

In the example below, the d10 rolls 8 and the object returned by DiceBox.roll() has the correct 'value' but for some reason the 'label' is set to "9".

{
    "notation": "d10",
    "sets": [
        {
            "num": 1,
            "type": "d10",
            "sides": 10,
            "rolls": [
                {
                    "type": "d10",
                    "sides": 10,
                    "id": 0,
                    "value": 8,
                    "label": "9",
                    "reason": "natural"
                }
            ],
            "total": 8
        }
    ],
    "modifier": 0,
    "total": 8
}

I looked at the code where it sets the label but I couldn't figure out why its doesn't use the same index into the value and label arrays around line 103 in DiceFactory.js:

    if (['d10','d2'].includes(this.shape)) matindex += 1;

    let value = diceobj.values[((matindex-1) % diceobj.values.length)];
    let label = diceobj.labels[(((matindex-1) % (diceobj.labels.length-2))+2)];

[lucasishuman]

Seeing some issues with d10s too.

Like here you can see I specified 2d10@8,1 in my notation, but the values are 5, 1 and labels are 6, 2

Other size dice seem ok so far. Except d100, but I'm not sure if that is currently supported...

This is such a great library - thanks!

[CapsE]

I was about to write that I can't reproduce the error yesterday but now I'm seeing it too. D10 are all over the place. Looks almost like the @ notation doesn't work for them. The Code-Sandbox example works perfectly fine though... I tried to recreate it as close as possible and still can't get it to work. I'm looking at your source code right now. I'll let you know if I find something.

[CapsE]

I realized why it was working yesterday (I think) I was using the library installed with yarn at version 0.0.6 where the d10 seems to work. Now I cloned the repo and copied the source (using 0.0.8) where the d10 is broken. That should make bug fixing easier because it's likely the issue is caused by a change that happened between those two versions. I tried looking into it but you changed a lot that could cause this bug.

I think let value = parseInt(dicemesh.getLastValue().value); in swapDiceFace(dicemesh, result) returns wrong (random) values causing this issue but I'm not a 100% sure. I returned right after that (not swapping faces) and the value I got didn't have any connection to the face being shown by the 3d model.

[frankieali]

This is the problem: https://github.com/3d-dice/dice-box-threejs/blob/2b429ff8d1a38392a7156abf774f665c416c5560/src/DiceFactory.js#L103-L106

Making a fix now

[frankieali]

Here it is. I'll bump the NPM release as well. Thanks for catching this issue. https://github.com/3d-dice/dice-box-threejs/blob/38c5ef16eefe9be443417066ff60da5ded48f8b5/src/DiceFactory.js#L94-L111

[frankieali]

NPM package updated to 0.0.9

[CapsE]

I just tried it. Sadly it still doesn't work.

{
    "notation": "1d10@9",
    "sets": [
        {
            "num": 1,
            "type": "d10",
            "sides": 10,
            "rolls": [
                {
                    "type": "d10",
                    "sides": 10,
                    "id": 0,
                    "value": 1,
                    "label": "1",
                    "reason": "forced"
                }
            ],
            "total": 1
        }
    ],
    "modifier": 0,
    "total": 1
}

The notation says is should get a 9 but I got a 1. It seems like I get random not matching results. The sides of the dice aren't linked to a specific result.

EDIT: I tried applying your changes to my files first but also installed the library with yarn (at 0.0.9) and tried without any of my changes.

[frankieali]

Yup, you're right. I'm seeing the issue as well. I was focused on fixing the labels. I didn't see that deterministic rolls for d10's was broken.

[frankieali]

ok, I think I fixed it properly this time. Just released version 0.0.11 on npm. You can also now initiate a roll with just the .add() method. Give it a try.

[frankieali]

Ok, the add method still had a bug. Fixed again. Fingers crossed this is the last thing. Release version 0.0.12

[CapsE]

Works like a charm! Thanks!