loathers / freecandydotexe

A trick-or-treating script for the 2003 browser-based RPG Kingdom of Loathing.
Apache License 2.0
6 stars 9 forks source link

fix: reset default grimoire preference #37

Closed midgleyc closed 11 months ago

midgleyc commented 11 months ago

The super(tasks); call in the constructor instantiates grimoire's default propertyManager that the declared propertyManager was shadowing; this means that in the destructor, super.destruct(); actually destroyed the shadowed one instead of the default one, and the defaultSettings in grimoire were assigned and never removed.

Among other things, this means that logPreferenceChange is set to true and never unset.

Move things around so that shouldn't happen.

midgleyc commented 11 months ago

Okay, that fixes the issue, but seems to break setting choice for wandered zones :/

horrible-little-slime commented 11 months ago

You need this.propertyManager = propertyManager as part of the constructor

midgleyc commented 11 months ago

If you have that, it shadows the grimoire-created one, which was already populated earlier in the constructor.

midgleyc commented 11 months ago

So, to be explicit about the issue:

  1. CandyEngine constructor runs
  2. this calls super() which means Engine constructor runs
  3. constructor calls this.initPropertiesManager(this.propertyManager); which populates the default property manager with default settings
  4. CandyEngine constructor later sets this.propertyManager to the static it declared earlier
  5. When the destructor runs, it calls Engine's destruct, which clears this.propertyManager.. but this clears the static, and not the default one, because it was overwritten in CandyEngine's constructor

Ideally I guess grimoire would expose a constructor for Engine that lets you pass in a property manager.

midgleyc commented 11 months ago

Obsoleted by #39.

horrible-little-slime commented 11 months ago

Superceded by #39, but we should make the aforementioned grimoire changes long-term