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

Debug Log fails after phase change #923

Closed
lkingsford opened this issue Mar 21, 2021 · 2 comments
Closed

Debug Log fails after phase change #923

lkingsford opened this issue Mar 21, 2021 · 2 comments

Comments

@lkingsford
Copy link

Expected behavior

When a game move goes to the next phase, and another move is made, the debug game log shows the actions taken.

Current behaviour

After a phase change, going to the log fails to show the moves made, and the JS application becomes unresponsive.

Reproduction:

Run the following app with Debug

(I used Webpack dev server)

app.js

import { Client } from 'boardgame.io/client';

const TestGame = {
    setup: () => ({ cells: Array(9).fill(null) }),

    phases: {
        initial: {
            start: true,
            moves: {
                clickCell: (G, ctx, id) => {
                    G.cells[id] = ctx.currentPlayer;
                },
                goToNextPhase: (G, ctx) => {
                    ctx.events.setPhase("next");
                }
            },
        },
        next: {
            moves: {
                doThing: (G, ctx, id) => {
                    G.cells[id] = null;
                }
            }
        }
    }
};

class GameClient {
    constructor() {
        this.client = Client({ game: TestGame });
        this.client.start();
    }
}

const app = new GameClient();

index.html

<!DOCTYPE html>
<html>
<body>
  <div id="app"></div>
</body>
</html>
  1. Use the click cell action
  2. Observe the action is logged
  3. Use the goToNextPhase action
  4. Use the doThing action
  5. Observe that the state changes as required
  6. Observe that the log now crashes

The following exception is logged in the debug console:

Debug-cf878fdc.js:5798 Uncaught (in promise) TypeError: (args || []).join is not a function
    at instance$p (Debug-cf878fdc.js:5798)
    at init (Debug-cf878fdc.js:636)
    at new LogEvent (Debug-cf878fdc.js:5852)
    at create_each_block_1 (Debug-cf878fdc.js:7013)
    at create_fragment$v (Debug-cf878fdc.js:7175)
    at init (Debug-cf878fdc.js:651)
    at new Log (Debug-cf878fdc.js:7518)
    at Object.p (Debug-cf878fdc.js:8699)
    at Object.p (Debug-cf878fdc.js:8872)
    at update (Debug-cf878fdc.js:368)

Additional Notes

I observed this initially not in the bug but in recreating the historic state from the log. I discussed it on March 7 at 17:34 (UTC+11) on the boardgame.io gitter.im instance. I also observed that it seems to be coming from a disallowed move - which the trace started with:

turn-order-391afcea.js:702 ERROR: disallowed move: removeCube
    error    @    turn-order-391afcea.js:702
(anonymous)    @    reducer-5a79a7da.js:871

In that reducer, conf.moves was undefined (hence returning null, hence disallowed) and state.ctx.phase was null.

I also note that the phase shift continues to let you play the game either server/client or hotseat - it only seems to effect recreating the state in certain circumstances (such as the debug log)

This is presently effecting the Emu Bay Railway Company online client - most significantly with the new branch letting you step through the game (which is, I think, crashing for this same reason).

@Dissolutio
Copy link
Contributor

Dissolutio commented Mar 27, 2021

I was able to recreate this bug using the React client instead. I got the same error as @lkingsford mentioned above, including how the game went on fine until I clicked the log. I don't know what this change breaks, but the following edit did fix the bug you encountered.

File and line mentioned in the exception: Debug-cf878fdc.js:5798 change:

// const renderedArgs = typeof args === "string" ? args : (args.toString() || []).join(",");
const renderedArgs = typeof args === "string" ? args : "";

I'm using bgio 43.3 and found the file at node_modules/boardgame.io/dist/esm/Debug-cf878fdc.js . After editing, I had to restart my webpack server for the changes to take effect.

I haven't looked into the other problems this was causing in your game, but hopefully this helps :) @lkingsford and great job on your game!!!

@delucis delucis closed this as completed in a7c8776 Apr 7, 2021
@delucis
Copy link
Member

delucis commented Apr 7, 2021

Thanks for the super detailed report @lkingsford! Sorry it took a while to get to it.

As @Dissolutio guessed, this line was not accounting for all values of args:

const renderedArgs = typeof args === 'string' ? args : (args || []).join(',');

Specifically, for that phase ending, args is an object {next: "next"}, which doesn’t have the Array.prototype.join method. I’ve fixed this in a7c8776.

A further problem is that the actual endPhase event is “automatic” (triggered from a move rather than directly), which should mean it doesn’t show up in the log pane at all, but it does, which suggests somewhere it’s being added to the game log without automatic: true. This means that during log replay, the move that ends the phase will run and then an additional end phase event will also be run (so end phase will happen twice), which I suspect is what is causing bugs in replay both here and in your history stepping UI @lkingsford.

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

No branches or pull requests

3 participants