Skip to content

Gamestate to POD#47

Merged
aliceagain merged 14 commits into
nextfrom
gamestate_to_pod
Sep 17, 2025
Merged

Gamestate to POD#47
aliceagain merged 14 commits into
nextfrom
gamestate_to_pod

Conversation

@aliceagain

Copy link
Copy Markdown
Member

No description provided.

@github-actions

github-actions Bot commented Aug 27, 2025

Copy link
Copy Markdown

Coverage Report

No coverage data available.

@github-actions

github-actions Bot commented Aug 27, 2025

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
JUnit Test Report156 ran156 passed0 skipped0 failed

@aliceagain aliceagain force-pushed the gamestate_to_pod branch 11 times, most recently from 2a42c09 to 0b40a35 Compare September 6, 2025 17:58
@aliceagain aliceagain force-pushed the gamestate_to_pod branch 5 times, most recently from a4e0df4 to ef21acc Compare September 16, 2025 02:14
@aliceagain aliceagain marked this pull request as ready for review September 16, 2025 02:24

@ChristopherJMiller ChristopherJMiller left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think everything lgtm! My understanding of the strategy is moving everything to be simple data inputs to produce mutations, instead of state + arbitrary indicator (like player slot) to calculate everything, which lets us generalize a bunch of things much better!

Found a very small nit that might be completely moot

state->concealedKan = false;
state->turnNum++;

Hand& hand = state->hands[state->currentPlayer];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: is this technically a double array access with the if statement above?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We update state->currentPlayer to state->lastCaller on line 38. So it's accessing a diff player now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah! got it

@aliceagain

Copy link
Copy Markdown
Member Author

And this is more about having the gamestate contain all of the state in one location instead of having additional heap allocations. Which means you can just memcpy the object itself without having to worry about it pointing to heap data.

It's not fully there but I have follow up stuff. Need to get the big diff out first tho.

@aliceagain aliceagain merged commit a50fde6 into next Sep 17, 2025
7 checks passed
@aliceagain aliceagain deleted the gamestate_to_pod branch September 17, 2025 02:30
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