Stop using GS.alsoDo() and overriding Goko methods

[aiannacc]

There are at least two good reasons not to use GS.alsoDo() or otherwise override native Goko methods if we could instead be listening to the Goko events directly.

First, it's easy to cause unintended and annoying problems for other Salvager modules that want to use the same Goko method. The kingdom generator causes complications of this sort by overriding FS.Dominion.DeckBuilder.Persistent.prototype.getRandomCards and FS.Dominion.DeckBuilder.Persistent.prototype._proRandomMethod. This balked my initial efforts to control when the KG popup appears and to implement Donald's 3-card veto mode.

Second, it means that if the Salvager code generates an Exception, it stops the native Goko code from running too. That's what's going on in #216. It's bad to have autokick break, but its much worse for that break to prevent anyone from joining your game at all.

We can usually avoid these problems by binding our code to the FS.Connection event producer instead of overriding Goko methods. The only challenge is that a lot of the events it produces are ambiguous or unintuitive. See eventIntermediary.js in the state-tracking branch for a listing and brief explanations of the most important events, as well as for my incomplete efforts to create an event producer that takes in Goko's events and spits out stuff that's easier to use.

[aiannacc]

To be clear, I don't think that fixing the existing usages should be given much priority. This is really just a best practice for new code.

[michaeljb]

Makes sense. I figured I could try to get in to the native Goko events with
a simple module that already existed rather than trying to dive right in
with something new--and I am actually working on something new! I'm sick of
the End Turn button and now I'm gonna do something about it.

On Tue, May 13, 2014 at 8:52 PM, Andrew Iannaccone <notifications@github.com

wrote:

To be clear, I don't think that fixing the existing usages should be given
much priority. This is really just a best practice for new code.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/217#issuecomment-43037189
.

Michael Brandt