Skip to content

AVR-1069: !init shuffle does not take ieffects into account#2200

Merged
codybanman merged 6 commits intoavrae:nightlyfrom
vaguely-happy:nightly
Oct 28, 2025
Merged

AVR-1069: !init shuffle does not take ieffects into account#2200
codybanman merged 6 commits intoavrae:nightlyfrom
vaguely-happy:nightly

Conversation

@vaguely-happy
Copy link
Copy Markdown
Contributor

@vaguely-happy vaguely-happy commented Jul 30, 2025

Summary

AVR-1069 !init shuffle does not account for ieffects

Changelog Entry

!init shuffle was updated to take ieffects into account

Checklist

PR Type

  • This PR is a code change that implements a feature request.
  • This PR fixes an issue.
  • This PR adds a new feature that is not an open feature request.
  • This PR is not a code change (e.g. documentation, README, ...)

Other

  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • If code changes were made then they have been tested.
  • I have updated the documentation to reflect the changes.

Copy link
Copy Markdown
Contributor

@1drturtle 1drturtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. See comment on possible improvement.

@7he4lph4
Copy link
Copy Markdown
Contributor

LGTM as well, the 3 line duplication around roll_str could be moved to the helper, and just watch out for flake8 errors.

Copy link
Copy Markdown
Contributor

@codybanman codybanman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good. I've added a suggestion but it mainly just needs cleanup to get passed the linter

Comment on lines +370 to +389
if isinstance(c, CombatantGroup):
gc = c.get_combatants()[0]
args = ieffect_handler(gc, argparse(""), "initiative")
b = args.join("b", "+", ephem=True)

# set up dice. Note that we still use the first combatant in the group for consistency with the ieffects
roll_str = gc.init_skill.d20(base_adv=args.adv(boolwise=True, ephem=True))

else:
args = ieffect_handler(c, argparse(""), "initiative")
b = args.join("b", "+", ephem=True)

# set up dice
roll_str = c.init_skill.d20(base_adv=args.adv(boolwise=True, ephem=True))

if b is not None:
roll_str = f"{roll_str}+{b}"

# roll
init_roll = roll(roll_str)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could clean up some of the redundancy by doing something like this.

Suggested change
if isinstance(c, CombatantGroup):
gc = c.get_combatants()[0]
args = ieffect_handler(gc, argparse(""), "initiative")
b = args.join("b", "+", ephem=True)
# set up dice. Note that we still use the first combatant in the group for consistency with the ieffects
roll_str = gc.init_skill.d20(base_adv=args.adv(boolwise=True, ephem=True))
else:
args = ieffect_handler(c, argparse(""), "initiative")
b = args.join("b", "+", ephem=True)
# set up dice
roll_str = c.init_skill.d20(base_adv=args.adv(boolwise=True, ephem=True))
if b is not None:
roll_str = f"{roll_str}+{b}"
# roll
init_roll = roll(roll_str)
combatant = c
if isinstance(c, CombatantGroup):
combatant = c.get_combatants()[0]
args = ieffect_handler(combatant, argparse(""), "initiative")
b = args.join("b", "+", ephem=True)
# set up dice. Note that we still use the first combatant in the group for consistency with the ieffects
roll_str = combatant.init_skill.d20(base_adv=args.adv(boolwise=True, ephem=True))
if b is not None:
roll_str = f"{roll_str}+{b}"
# roll
init_roll = roll(roll_str)

@codybanman codybanman changed the title AVR-1069 AVR-1069: !init shuffle does not take ieffects into account Sep 22, 2025
@codybanman codybanman merged commit c6849ce into avrae:nightly Oct 28, 2025
codybanman added a commit that referenced this pull request Oct 28, 2025
codybanman added a commit that referenced this pull request Oct 28, 2025
7he4lph4 pushed a commit to 7he4lph4/avrae that referenced this pull request Nov 18, 2025
7he4lph4 pushed a commit to 7he4lph4/avrae that referenced this pull request Nov 18, 2025
@codybanman codybanman mentioned this pull request Nov 25, 2025
5 tasks
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

Successfully merging this pull request may close these issues.

4 participants