Skip to content

Comments

WIP: Onboarding#543

Open
Themezv wants to merge 9 commits intomasterfrom
onboarding2
Open

WIP: Onboarding#543
Themezv wants to merge 9 commits intomasterfrom
onboarding2

Conversation

@Themezv
Copy link
Member

@Themezv Themezv commented Oct 26, 2025

Note

Medium Risk
Adds a new onboarding state machine wired into Redux listener middleware and the main router, affecting app state and navigation behavior. Risk is moderate due to new async listener loop and UI gating logic, though changes are localized to the onboarding path.

Overview
Adds a new /onboarding route and a Redux onboarding slice backed by an @fsmoothy/core finite-state machine, with listener middleware that boots the FSM on route entry and advances it based on dispatched onboarding actions.

Updates the onboarding page to render a tutorial-specific game board from the FSM state, including step-based layering, a timed touch hint, and a custom “Next” button; also adds card highlighting/disable support (Card, LigrettoPack) and makes CardsPanel composable via children.

Written by Cursor Bugbot for commit 94ec220. This will update automatically on new commits. Configure here.

cursor[bot]

This comment was marked as outdated.

@github-actions
Copy link

github-actions bot commented Oct 26, 2025

Coverage report for apps/ligretto-gameplay-backend

St.
Category Percentage Covered / Total
🔴 Statements 49.8% 373/749
🔴 Branches 25.24% 26/103
🔴 Functions 26.7% 55/206
🔴 Lines 47.47% 310/653

Test suite run success

12 tests passing in 1 suite.

Report generated by 🧪jest coverage report action from 94a6762

@Themezv Themezv changed the title Onboarding2 WIP: Onboarding Nov 1, 2025
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 5 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on March 23

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

},
effect: async (_action, listenerApi) => {
const fsm = new OnboardingStateMachine()
console.log(render(fsm.inspect()))
Copy link

Choose a reason for hiding this comment

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

Debug console.log and graphviz import left in code

Medium Severity

console.log(render(fsm.inspect())) renders the entire FSM as a graphviz diagram to the browser console every time the user navigates to the onboarding page. The render import from @fsmoothy/graphviz and the corresponding package dependency in package.json exist solely for this debug statement, unnecessarily increasing the production bundle size.

Additional Locations (1)

Fix in Cursor Fix in Web

if (fsm.current === OnboardingStep.Result) {
break
}
}
Copy link

Choose a reason for hiding this comment

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

Missing cancelActiveListeners causes duplicate infinite loops on re-navigation

High Severity

The listener effect contains a while (true) loop with listenerApi.take() but never calls listenerApi.cancelActiveListeners() at the start. Each navigation to the onboarding route starts a new effect invocation without cancelling previous ones. Stale loops from prior visits continue running in the background, each with their own FSM instance, each dispatching competing setOnboardingState actions. The existing game listener in ducks/game/listeners.ts correctly calls cancelActiveListeners() to prevent this exact problem.

Fix in Cursor Fix in Web

},
},
results: undefined,
}
Copy link

Choose a reason for hiding this comment

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

Slice initial state differs from FSM initial data

Medium Severity

The Redux slice initialState for player id0 has cards [{5, blue}, {5, blue}, {5, blue}] with one ligretto card, but the FSM data() initializes id0 with cards [{1, blue}, {3, blue}, {3, green}] and three ligretto cards. Since the FSM is the source of truth driving the onboarding logic, the Redux state will briefly show incorrect card data on first render before the listener dispatches the FSM state.

Additional Locations (1)

Fix in Cursor Fix in Web

onLeave: (ctx: OnboardingContext) => {
ctx.data.isCycledInfoShown = true
},
}
Copy link

Choose a reason for hiding this comment

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

Shared flag makes OpponentTurnCycledInfo state unreachable

Medium Severity

The cycledInfoHooks guard references the single shared isCycledInfoShown flag, which is set to true when leaving GameStartedCycledInfo. By the time the FSM reaches OpponentTurn, the flag is already true, so the guard !ctx.data.isCycledInfoShown always fails and the transition to OpponentTurnCycledInfo is never taken. Despite having an explicit state and transitions defined, OpponentTurnCycledInfo is completely unreachable.

Additional Locations (1)

Fix in Cursor Fix in Web

OnboardingStep.StackAvailableCard,
OnboardingStep.StackCard,
OnboardingStep.StackUnavailableCard,
OnboardingStep.StackAvailableCard,
Copy link

Choose a reason for hiding this comment

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

Duplicate StackAvailableCard entries in layer visibility sets

Low Severity

OnboardingStep.StackAvailableCard appears twice in both the playerCards and playgroundCards Sets. Since these are Set objects, the duplicates are silently deduplicated with no runtime error, but the identical pattern in both sets (appearing before StackCard/StackUnavailableCard and again after) strongly suggests one entry was intended to be a different step and was copy-pasted by mistake.

Additional Locations (1)

Fix in Cursor Fix in Web

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.

2 participants