aiannacc / Goko-Salvager

Enhance your Dominion Online experience!
13 stars 9 forks source link

Kingdom Generator needs major overhaul #91

Open aiannacc opened 11 years ago

aiannacc commented 11 years ago

There are several problems with the current implementation:

serakfalcon commented 11 years ago

I'm going to separate the set parser and data stuff into a "mini-library" to be called kinggen_utils, before the 'work code' in kingdomgenerator.js (to be separated out once I'm sure there's no side-effects and I have a better handle of how everything comes together).

I'll also separate out the displayed HTML into its own section, it could also potentially be stored separately since the 'visible' part only takes up 1 line of code right now.

serakfalcon commented 11 years ago

OK my latest commit should fix

To Do:

aiannacc commented 11 years ago

Great fixes and proposals!

A few thoughts and some possibly-useful advice:

It would be nice to have an error message when you try to create an illegal kingdom. Right now the UI just doesn't respond.

I suggest getting rid of the innerHTML anyway, since we don't want to count on the Mozilla folks looking into our code to realize that it's not actually dangerous. Then again, releasing to the Firefox store isn't going to be an option anytime soon anyway, since the Mozilla add-on review process seems to have ground to a complete halt.

Running VerifyCardIds on every new game (and worse, every time the create game dialog is opened) is totally unnecessary. Some variant of it should run once on login, and maybe again after visiting the Goko store, and the information should be cached. The method itself should be overridden with a dummy that just checks the cache.

Implementing Pro Veto Mode in the kingdom generator sounds tricky. You have to interact with your opponent somehow, probably through a third party server. You're welcome to use mine, but you'll have to install and set up the environment for goko-dominion-tools in order to make changes. If you do, you may find it helpful to look to the automatch code to see how the WebSocket communication works. Some parts of the code are pretty ugly, particularly the disconnect detection, but it'll show you how it works. I suggest creating a second WS connection rather than abusing the existing automatch connection to add the veto-mode communication.

There's also a sneaky bit of Goko code for pro games to watch out for. An initial kingdom gets generated when you create the game. It uses the same method that generates a casual/unrated kingdom regardless of whether you're creating a pro/casual/unrated game. But for pro games, a second kingdom gets generated right before the game starts using a different method, and this is the kingdom that the actual game uses. The kingdom generator overrides the casual/unrated kingdom creation but not the pro.

Kingdom generation and game creation is a particularly nasty section of the Goko code. You can delve into it and try to make the exact changes you want, as the kingdom generator does now, but I think the easier solution might simply be to ignore it entirely, altering the create/edit game and start game UI elements to call our own code and communicate with the server directly instead. There's code elsewhere in Salvager that shows how to specify the kingdom and create a game table directly, so you just have to mock a Goko client's request to the server start a game and (possibly) intercept the server's responses.

There used to be a separate file called set_parser.js that contains just the machine-generated Jison set parser. I rather idiotically merged it into the kingdom generator script. You might save yourself some time by retrieving it from one of the old tags.