CCSE bug: `CCSE.NewUpgrade` breaks the closure of `buyFunction`

[staticvariablejames]

CCSE.NewUpgrade calls CCSE.ReplaceUpgrade, which runs CCSE.SliceCodeIntoFunction in the function passed as the argument buyFunction. This breaks closures.

For example, the following code snippet is broken:

function myInit() {
  let sentence = 'Hi!';
  let myBuyFunction = function() {console.log(sentence);}
  CCSE.NewUpgrade('Hello world', 'Says "hi" on the console', 7, Game.Achievements['F12'].icon, myBuyFunction);
  Game.Unlock('Hello world');
}
myInit();

---

Why is even CCSE.ReplaceUpgrade being run in the first place? That function is meant to inject hooks to allow modders to modify vanilla upgrades with minimal conflict between each other. But this is a modded upgrade; I (the modder) am responsible for providing hooks for that upgrade.

[klattmose]

I guess it's a choice between expecting a mod author to put a hook in their upgrades or expecting them to write their functions so that they don't break this way.

Is this breaking an upgrade in a real mod for you or are you being proactive?

[staticvariablejames]

Yes. Currently, I'm working around this issue by overwriting buyFunction after creating the upgrade: https://github.com/staticvariablejames/SpicedCookies/blob/3fc5df802fac02c9d74fc019dc72a9a2521693cb/src/modules/stock-market/extra-rows.ts#L122

In my case, buyFunction calls another function, which is not visible in the global scope.

In general, I think it is a bad idea to rewrite functions, especially if their code is not known a priori. We do it with Orteil's code because we have no other reasonable option. For example, I recently ported Spiced Cookies to TypeScript, so the buyFunction that I feed to CCSE is compiled; I have no control over how the code actually looks like. Another example (that does not affect me at the moment, but I can see being an issue nonetheless) is arrow functions:

function myInit() {
  let myBuyFunction = () => console.log('Hi!');
  CCSE.NewUpgrade('Hello world', 'Says "hi" on the console', 7, Game.Achievements['F12'].icon, myBuyFunction);
  Game.Unlock('Hello world');
}
myInit();

Now the code of buyFunction does not fit into the mold expected by CCSE.ReplaceUpgrade, so the code rewrites are invalid and CCSE crashes.

[klattmose]

What if I wrap that function instead? The current version in beta (as of 5 minutes ago) handles both of your demo functions adroitly.

[staticvariablejames]

Yep, that solves the problem!

It introduces a theoretical problem that might affect us down the line (but I think no implications for mods that exist today): Your patch modifies CCSE.ReplaceUpgrade, which is called on initialization to also modify vanilla upgrades. I don't like using wrapping here for the same reason that Cookie Monster must be loaded after CCSE: after some mod wraps some function, it is impossible to perform code injections. This may later introduce incompatibilities with mods that e.g. use Cppkies, or do injections directly (like Frozen Cookies, and even Cookie Monster itself).

I'm closing this issue, but I think that the theoretical problem above should be kept in mind.