cameroncondry / cbc-kitten-scientists

Add-on for the wonderful incremental browser game: http://kittensgame.com/web/
MIT License
113 stars 94 forks source link

Fixing The Enable-All Button/At least one Trade Error #248

Closed Wymrite closed 5 years ago

Wymrite commented 5 years ago

I found the cause of the long-standing enable-all bug, where clicking enable-all would enable everything but then instantaneously disable it referenced in #155 , #160 , and possibly #159. It appears that the cause was that enabling used .is() while disabling used .not(), and while .is() leaves the property intact, .not() deletes it if it finds it. The enable-all and disable-all buttons would call these twice at one point, so clicking enable-all would have .not(':checked') return false, delete the 'checked' property, then .not(':checked') would return true and it would toggle the buildings off. At least that is my understanding of what was going on. Regardless, changing the input.not() to !input.is() fixed the issue.

Wymrite commented 5 years ago

Found the likely cause of the trade error where it mysteriously stops working and throws a bunch of errors in the console. The tab manager was only getting rendered upon creation of a trade manager rather than each time the the KS cycle would call trade, which apparently made it so the trade manager would think another tab is the trade menu sometimes and also would cause it to break whenever leviathans would arrive or leave. This is almost certainly the issue behind #139, #170, and #206.

Notably, changing it to render on each iteration appears to have also made "trade trigger = 0" #153 constantly trade as it should, though I would not recommend this as the trade tab currently is extremely resource intensive. When monitoring it with Firefox's console, I would get 55 fps without KS, 50 fps with all KS settings except trade and exploration enabled including the auto-upgrading in my other pull request, 30 fps with leviathans and zebras set to a trigger of 0.2, and between 0 and 2 fps with all trade settings enabled and a trigger of 0. How the trade automation functions definitely needs a re-evaluation.

cameroncondry commented 5 years ago

Very good catch! Thank you for your continued work on keeping the script updated.