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

Problem with LocalMaster using previous instantiation of a Client #552

Closed
julienroulle opened this issue Mar 4, 2020 · 5 comments
Closed

Comments

@julienroulle
Copy link
Contributor

I've had some trouble with the Local use of the lib.

When I first instantiate a Local Game like that with bots coming from another screen:

bots = {'0': MyBot}
var GameApp = new Client({
        game: MyGame,
        board: MyBoard,
        multiplayer: new Local({
          bots,
        }),
      });

When going back to my "parameters screen" and changing the turn of my bot, meaning changing the following var:

bots: {'1': MyBot}

I discovered that my new GameApp will use the previous parameters of my 1st instantiation.

I managed to find the cause of the problem, but I'm not sure why this is causing a problem.
It comes from this if-else statement.

By commenting out the first block of the if-statement, I managed to make it work as I wanted to be (meaning instantiating a new Local Master with my new given parameters). But as I am not really familiar with this part of the lib, I'm not sure what this if-statement is here for, and what damaged I could do by removing it.

Please share with me your knowledge about that if possible :)

@julienroulle
Copy link
Contributor Author

Looks like commenting out this block breaks a test for Local Multiplayer without bots ..
So I don't know yet how to tackle this problem.

@julienroulle
Copy link
Contributor Author

I managed it make it work by replacing the following:

const localMasters = new Map();
export function Local(opts) {
  return transportOpts => {
    let master;

    if (localMasters.has(transportOpts.gameKey)) {
      if (
        localMasters
          .get(transportOpts.gameKey)
          .storageAPI.games.has(transportOpts.gameID)
      ) {
        master = localMasters.get(transportOpts.gameKey);
      } else {
        master = new LocalMaster({
          game: transportOpts.game,
          bots: opts && opts.bots,
        });
        localMasters.set(transportOpts.gameKey, master);
      }
    } else {
      master = new LocalMaster({
        game: transportOpts.game,
        bots: opts && opts.bots,
      });
      localMasters.set(transportOpts.gameKey, master);
    }

    return new LocalTransport({ master, ...transportOpts });
  };
}

But for some reasons it makes the multiplayer interactions test of client.test.js fail ..

@julienroulle
Copy link
Contributor Author

Doing this:

const localMasters = new Map();
export function Local(opts) {
  return transportOpts => {
    let master;

    if (localMasters.has(transportOpts.gameKey)) {
      if (!opts) {
        master = localMasters.get(transportOpts.gameKey);
      } else {
        master = new LocalMaster({
          game: transportOpts.game,
          bots: opts && opts.bots,
        });
        localMasters.set(transportOpts.gameKey, master);
      }
    } else {
      master = new LocalMaster({
        game: transportOpts.game,
        bots: opts && opts.bots,
      });
      localMasters.set(transportOpts.gameKey, master);
    }

    return new LocalTransport({ master, ...transportOpts });
  };
}

Also solves the problem, and makes the test pass. But i doubt it's an effective way to solve the problem ...
Please give me feedback on how I could investigate this issue further, I'm out of idea.

@nicolodavis
Copy link
Member

That's a fine way to solve the problem (your last update). Perhaps just consolidate the duplicated code in the two else branches.

Would you mind sending a PR for this?

@julienroulle
Copy link
Contributor Author

Yes I was thinking about consolidating it as soon as I'd get feedback if I was on the right way or not!
I'll send you the PR later :)
Thanks for answering!

@julienroulle julienroulle changed the title Problem with LocalMaster using previous instantitation of a Client Problem with LocalMaster using previous instantiation of a Client Mar 5, 2020
@julienroulle julienroulle mentioned this issue Mar 5, 2020
2 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

No branches or pull requests

2 participants