CookieMonsterTeam / CookieMonster

Addon for Cookie Clicker that offers a wide range of tools and statistics to enhance the game
MIT License
500 stars 206 forks source link

Incompatible with some content mods #560

Closed hyoretsu closed 3 years ago

hyoretsu commented 3 years ago

When using a mod that modifies CpS in any "non-standard" (injecting into vanilla functions) way, such as Cookie Clicker OC Ideas's Misfortune tier or Saffronyx tier cursor upgrade, Bonus Income and Payback Period calculation stops working. Before purchasing Misfortune 502 image After image For some reason it only happens for some misfortunes, e.g. Misfortune 503 doesn't do this, even though they all modify CpS with the same code. (Not even re-used code, they all come from the same lines of logic)

I suspect it's because of the pretty much useless (and static) Sim.js functions.

DanielNoord commented 3 years ago

You seem to have implemented misfortunes by injecting additional code into Game.GetTieredCpsMult. After loading your mod the following line is added to that function:

if(me.vanilla===1&&Game.Has(me.tieredUpgrades.misfortune.name)){switch(Game.elderWrath){case 1:mult*=1.02;break;case 2:mult*=1.04;break;case 3:mult*=1.06;break;}}

CookieMonster works by (for example) loading the current data, adding one cursor building and then running all calculations. This is why we have static functions in Sim.js that check whether certain conditions are met in the current "sim data" (the one including the additional building). This means that we can not really account for such injections into vanilla functions. While we could use eval to load and replace Game. with CM.Sim in the vanilla functions this would still create problems whenever a content mod is loaded after CookieMonster.

I don't really see an easy solution for this problem, but as you are certainly more experienced with JS you might know of a solution! I am open to any suggestions!

hyoretsu commented 3 years ago

A possible solution would be simply using Game.GetTieredCpsMult. CM.Sim.GetTieredCpsMult is literally a copy of it, with a few parts changed to adapt to the whole simulation scenario. (me.fortune switched to Game.Objects[me.name].fortune) Imo it's a complicated mess that was needlessly introduced in the past. One such way to partially support that would be to assign the Sim functions with an actual replacing. As eval isn't really advised to use, (That's an ESLint rule, at least in airbnb-base, and a consensus) just do it like:

CM.Sim.GetTieredCpsMult = new Function(
    `return ${Game.GetTieredCpsMult.toString()
    .split('syn.buildingTie1.amount')
    .join('CM.Sim.Objects[syn.buildingTie1.name].amount')
    .split('syn.buildingTie2.amount')
    .join('CM.Sim.Objects[syn.buildingTie2.name].amount')}`,
 )();

That's at least a partial (working) support accounting for modifications made in mods loaded prior to CM, which is better than no support at all. It's also way cleaner and smaller. (Yes, changing vanilla functions to CM.Sim ones and me to Game.Objects[me.name] is useless in this case, and probably in many many others)

CM.Sim.Has is literally the vanilla function with CM.Sim.Upgrades. CM.Sim.Upgrades is literally a .forEach() in Game.Upgrades ran through CM.Sim.InitUpgrade. CM.Sim.InitUpgrade is literally Game.Upgrades[building] with only pool, power and name and if power is a function, replacing Game.Has and Game.hasGod. Useless repetition. Another testament to my "the majority of Sim.js is pointless" point is that you don't need to do this in CM.Sim.InitialBuildingData (only place where Game.GetTieredCpsMult is used)

eval('you.cps = ' + me.cps.toString()
    .split('Game.Has').join('CM.Sim.Has')
    .split('Game.hasAura').join('CM.Sim.hasAura')
    .split('Game.Objects').join('CM.Sim.Objects')
    .split('Game.GetTieredCpsMult').join('CM.Sim.GetTieredCpsMult')
    .split('Game.auraMult').join('CM.Sim.auraMult')
    .split('Game.eff').join('CM.Sim.eff')
);

The only things you gotta replace are

 you.cps = new Function(
  `return ${me.cps.toString()
   .split('Game.Objects').join('CM.Sim.Objects')
   .split('Game.GetTieredCpsMult').join('CM.Sim.GetTieredCpsMult')}`
 )();

And only because you need to change CM.Sim.Objects in CM.Sim.GetTieredCpsMult, so that's one of the (few) needed things from Sim.js.

The way things are done, it wouldn't only break with Misfortune, but with any custom tier that adds a cursor upgrade, as you need to inject to Game.mouseCps and Game.Objects.Cursor.cps for that. Or anything that injects to a CpS-related function at that. (Next tiers from my mod will all have cursor upgrades)

hyoretsu commented 3 years ago

I fixed it, pretty simple actually. Lemme make a PR real quick.

DanielNoord commented 3 years ago

I was indeed trying to remove all eval's and ran into some trouble with replacing them by new Functions (although this was weeks ago). Therefore I have kept some of them. I might need to take another look!

However, I don't think your reduced you.cps function is correct. For example, the .cps method of the Cursor object includes the line mult*=Game.eff('cursorCps'). If we want to be able to simulate the correct calculation of a change that affects the 'cursorCps' effect (for example harvesting a certain plant) we will need to make sure that every function used by CM.Sim.CalculateGains() is using CM.Sim versions. This is why the current replacements are so extensive in CM.Sim.InitUpgrade(), CM.Sim.Has(), etc. Obviously this is not my own code (and you are much better at JS) so I am never 100% sure but I think your suggested change will break or make the current simulation less accurate.

hyoretsu commented 3 years ago

I checked Game.eff for cursor (planted a field full of Glovemorels) and values with/without replacing Game.eff were the same. It's as simple as repeating that line if it's actually needed though. Besides, you can still use the same logic I used in PR to modify them in a non-intrusive (static) way and at least reduce the amount of CM.Sim you have to use (Given the useless ones are removed first, and only then the remaining, needed ones injected to vanilla functions to not break any other mods and keep consistency throughout CC)