Chromatic Orb difficult to cast with advantage / disadvantage

[matthewbstroud]

Describe the bug Trying cast chromatic orb with advantage launches a damage type dialog, which doesn't automatically honor the advantage/disadvantage modifier.

To Reproduce Steps to reproduce the behavior:

1. Have Chromatic Orb in your spell list
2. Hold down shift and click the attack roll.
3. A popup will show asking for damage type.
   
4. Choose a damage type and click okay.
5. A single d20 will roll.

Expected behavior You should only be prompted for a damage type when rolling damage for chromatic orb, since if you miss the damage is meaningless. Or Clicking the attack roll should honor the advantage/disadvantage modifiers.

Workaround Holding shift when you click the Roll button in the dialog does make the advantage roll work.

Browser Info (please complete the following information):

• Edge
• Version 105.0.1343.53 (Official build) (64-bit)

Errors

[kakaroto]

Nice find, thanks for the report. The popup dialog is definitely interfering here!

[Aeristoka]

To clarify, no, it should absolutely do the dialogue to specify damage here, since it's going to give you the "Roll Damages" button, as it does when you use DnDBeyond Digital Dice. If it didn't do it here, we've lost the easy facility to do that.

Yes, it absolutely should respect advantage all the way through.

the utils.ts error is unrelated to Beyond20.

[matthewbstroud]

[Aeristoka]

Context of what you were doing when you saw this?

[matthewbstroud]

Clicking on the Chromatic Org to Hit button over and over.

[matthewbstroud]

so, this seems to fix the issue

async function buildAttackRoll(character, attack_source, name, description, properties,
                         damages = [], damage_types = [], to_hit = null,
                         brutal = 0, force_to_hit_only = false, force_damages_only = false, {weapon_damage_length=0}={}) {
    const roll_properties = {
        "name": name,
        "attack-source": attack_source,
        "description": description,
        "rollAttack": force_to_hit_only || !force_damages_only,
        "rollDamage": force_damages_only || (!force_to_hit_only && character.getGlobalSetting("auto-roll-damage", true)),
        "rollCritical": false
    }
    // store the key modifier in the request prior to launching any dialog
    if (key_modifiers.advantage)
        roll_properties["advantage"] = RollType.ADVANTAGE;
    else if (key_modifiers.disadvantage)
        roll_properties["advantage"] = RollType.DISADVANTAGE;
    if (key_modifiers.super_advantage)
        roll_properties["advantage"] = RollType.SUPER_ADVANTAGE;
    else if (key_modifiers.super_disadvantage)
        roll_properties["advantage"] = RollType.SUPER_DISADVANTAGE;
    else if (key_modifiers.normal_roll)
        roll_properties["advantage"] = RollType.NORMAL;

async function sendRoll(character, rollType, fallback, args) {
    let whisper = parseInt(character.getGlobalSetting("whisper-type", WhisperType.NO));
    const whisper_monster = parseInt(character.getGlobalSetting("whisper-type-monsters", WhisperType.YES));
    let is_monster = character.type() == "Monster" || character.type() == "Vehicle";
    if (is_monster && whisper_monster != WhisperType.NO)
        whisper = whisper_monster;
    // Let the spell card display appear uncensored
    if (rollType === "spell-card" && whisper === WhisperType.HIDE_NAMES)
        whisper = WhisperType.NO;

    advantage = parseInt(character.getGlobalSetting("roll-type", RollType.NORMAL));
    if (args["advantage"] == RollType.OVERRIDE_ADVANTAGE)
        args["advantage"] = advantage == RollType.SUPER_ADVANTAGE ? RollType.SUPER_ADVANTAGE : RollType.ADVANTAGE;
    if (args["advantage"] == RollType.OVERRIDE_DISADVANTAGE)
        args["advantage"] = advantage == RollType.SUPER_DISADVANTAGE ? RollType.SUPER_DISADVANTAGE : RollType.DISADVANTAGE;

    // Default advantage/whisper would get overriden if (they are part of provided args;
    const req = {
        action: "roll",
        character: character.getDict(),
        type: rollType,
        roll: cleanRoll(fallback),
        advantage: advantage,
        whisper: whisper
    }
    for (let key in args)
        req[key] = args[key];
    // if advantage has already been set, use the value
    if (args["advantage"]) {
        req["advantage"] = args["advantage"];
    } else if (key_modifiers.advantage)
        req["advantage"] = RollType.ADVANTAGE;
    else if (key_modifiers.disadvantage)
        req["advantage"] = RollType.DISADVANTAGE;
    if (key_modifiers.super_advantage)
        req["advantage"] = RollType.SUPER_ADVANTAGE;
    else if (key_modifiers.super_disadvantage)
        req["advantage"] = RollType.SUPER_DISADVANTAGE;
    else if (key_modifiers.normal_roll)
        req["advantage"] = RollType.NORMAL;

[Aeristoka]

Would help more to submit a Pull Request.

[matthewbstroud]

Okay, let me see if i can figure out how to do that.
The issue is a race condition.

• User holds down shift, which sets key_modifiers.
• Chromatic orb launches a dialog
• User releases shift
• key_modifiers is updated
• Dialog is completed
• SendRoll reads key_modifiers, which no longer indicates advantage.

The solution is to store the key modifiers in roll_properties prior to launching the dialog.

[matthewbstroud]

I do not have access to make a PR

[Aeristoka]

You absolutely do. You fork the Repo, make the branch on your fork, then submit it as a PR.

[matthewbstroud]

Okay, give me a minute.

[matthewbstroud]

@Aeristoka for the PR, should it be to a branch or main?

[Aeristoka]

I personally ALWAYS do a branch, and keep my Main synced with upstream. It's a lot cleaner.

[matthewbstroud]

https://github.com/kakaroto/Beyond20/pull/1023/files

[matthewbstroud]

My fix just solves a race condition, but it fails the principle of DRY. I could move the logic to get the advantage from key_modifiers into its own function, but I wanted to see if I was even in the ballpark.

[matthewbstroud]

@Aeristoka thanks for the help, this is my first time forking something.