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

New roll engine #422

Merged
merged 128 commits into from
Aug 24, 2022
Merged

New roll engine #422

merged 128 commits into from
Aug 24, 2022

Conversation

ben
Copy link
Owner

@ben ben commented Aug 8, 2022

This is to address #363: a new dialog and a helper class that can encapsulate all the logic involved in preparing, rolling, and modifying the results of Ironsworn/Starforged rolls.

  • New IronswornRoll class, which performs rolls, renders to chat cards, and captures the state of the roll for modification afterwards
  • New dialog class, for preparing for and executing a roll
  • Allow for after-roll modifications via chat-card buttons or another dialog
  • Update CHANGELOG.md
  • Wait for semantics fixes for new-roll-engine #436 to merge into this

Things I'm leaving for other PRs:

  • A dialog for the user to resolve extra challenge dice
  • Suggestions from character assets
  • A new visual look for rolling as described by Revamped roll-result output #366

src/module/rolls/roll.ts Outdated Show resolved Hide resolved
export interface PreRollOptions {
automaticOutcome?: HIT_TYPE
presetActionDie?: number // As in Armored #1
threeChallengeDice?: boolean // As in Sleuth #1
Copy link
Collaborator

Choose a reason for hiding this comment

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

here's one possible situation where the numeric MOVE_OUTCOME enum described above might be useful in some respects.

that style works best in situations where the challenge dice live in an array or similar, e.g.

const challengeDice: [number, number] = [1, 2]

because array methods make running the comparison pretty clean compared to a series of if/thens for the specific property representing each challenge die.

the presence of more than two dice in that array could instead pass those dice to a reduce-like function that selects the dice being used, encapsulating the logic for choosing the challenge dice that are used and throwing an error if no such function is provided.

that might make it easier to extend for other dice selection methods in the future, too.

Copy link
Owner Author

@ben ben Aug 9, 2022

Choose a reason for hiding this comment

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

Sleuth #1 says "roll three challenge dice and choose two." That doesn't mean "always use the two lowest outcomes"; I could definitely see a 4 vs 3/6/6 situation, where it's a choice between a weak hit and a very interesting miss, or a 5 vs 2/3/3, where the mathematically-sorted outcome may not be the most interesting one.

So this specific flag isn't a 5e-style "advantage" mechanic, it indicates that once the dice are rolled, we'll need to let the player decide which ones to keep, which is probably an interstitial dialog, or some kind of interesting chat-card UI.

I should probably make the number of dice more generic, though. Maybe this should be called extraChallengeDice?: number.

Copy link
Collaborator

Choose a reason for hiding this comment

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

[apparently i accidentally edited your message instead of quote replying 🤦 i've reverted to the original comment, sorry about that!]

Sleuth #1 says "roll three challenge dice and choose two." That doesn't mean "always use the two lowest outcomes"; I could definitely see a 4 vs 3/6/6 situation, where it's a choice between a weak hit and a very interesting miss, or a 5 vs 2/3/3, where the mathematically-sorted outcome may not be the most interesting one.

So this specific flag isn't a 5e-style "advantage" mechanic, it indicates that once the dice are rolled, we'll need to let the player decide which ones to keep, which is probably an interstitial dialog, or some kind of interesting chat-card UI.

ah, whoops, fair enough on Sleuth specifically. i think i got some wires crossed with Loyalist. 🤦 which can get a bit more mechanistic re: die replacement, but only under certain circumstances, and it's complicated by several other factors... so it's probably outside the scope of this particular thread, haha.

FWIW - for a situation like sleuth 3, it might be possible to use a similar (but less automated) workflow: pass to a function could still apply and encapsulate the selection process. except instead of something mechanistic/reduce-like, it'd be a function that presents a selection dialog and await the player's input (probably with some feedback of the initial roll outcome to the chatlog of the dice results for record-keeping purposes), then proceeds with roll resolution. something like:

You rolled 3 challenge dice. Result: 4, 1, 7
Please select two challenge dice to use:

the alternative, i suppose, would be the user completing that selection by invoking a similar dialog from something embedded in the roll output itself. but if someone's going to the trouble of advanced challenge die configuration, i think it'd be reasonable to automatically present the selection dialog; presumably they're rolling because they want a move outcome, and that resolution is bottlenecked until additional input is provided - might as well put it right in front of them and save some effort.

that might be a good way to address it in a way that's more versatile for handling whatever weird homebrew dice replacement mechanics people want to come up with: have the player pick manually if there's more than two challenge dice present before proceeding. and i reckon it'd fit much more naturally with the general ethos of this foundry module than automating it to the degree i suggested earlier, ha

src/module/rolls/roll.ts Outdated Show resolved Hide resolved
@@ -126,7 +127,7 @@ function clearProgress() {
}

function markProgress() {
const increment = CONFIG.IRONSWORN.RankIncrements[foe.value?.data.rank]
const increment = RANK_INCREMENTS[foe.value?.data.rank]
const newValue = Math.min(foe.value?.data.current + increment, 40)
Copy link
Collaborator

@rsek rsek Aug 8, 2022

Choose a reason for hiding this comment

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

would it be worth making the 40 a reference to a constant throughout the module rather than a "magic number"? mainly to make the purpose of the code really clear at a glance. perhaps something like the following:

const TICKS_PER_BOX = 4
const PROGRESS_MAX_BOXES = 10
const PROGRESS_MAX_TICKS = PROGRESS_MAX_BOXES * TICKS_PER_BOX

@rsek rsek linked an issue Aug 8, 2022 that may be closed by this pull request
@ben ben force-pushed the new-roll-engine branch 3 times, most recently from 2f1dba9 to ada5953 Compare August 10, 2022 17:45
@ben ben force-pushed the new-roll-engine branch from ada5953 to 964f16f Compare August 11, 2022 13:21
@ben
Copy link
Owner Author

ben commented Aug 24, 2022

Nice. Almost there:

CleanShot 2022-08-24 at 02 33 36

@ben ben merged commit c8ea864 into main Aug 24, 2022
@ben ben deleted the new-roll-engine branch August 24, 2022 14:45
@ben ben mentioned this pull request Aug 28, 2022
7 tasks
@ben ben mentioned this pull request Sep 5, 2022
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.

Enhanced roll flow
2 participants