Undo doesn't refund material
I have witnessed several times materials not refunded after clicking on 'undo' for a craft/purchase. Wondered if it was a bug or an intended behavior?
Comments (9)
-
-
After checking the code and its history, it appears that the triggering of the “undo” button for Bonfire building purchase has been added more than 2 years ago (Sept. 2016) in a pull request that was totally unrelated with that subject, but the actual implementation of what to do when the “undo” button is pressed has never been coded, so it has never worked. So, for now, the more logical thing to do is to remove the useless “undo” button for Bonfire buildings (so nothing to do for 2/ for now). So only 3/ needs a fix.
BTW while checking the code it appears that everything is ready for undoing several crafts (craft A, immediately craft B, undo, B and A should be uncrafted), but actually each new craft overrides the previous one (craft A, immediately craft B, undo, only B is uncrafted) instead of being appended. As everything above has been coded in a single commit with a comment saying “I am too tired to write proper logic”, it really looks like the initial intention was undo chaining and that undo override is a bug.
-
@bloodrizer In
game.js
:/** * Undo Change state. Represents a change in one or multiple */ dojo.declare("classes.game.UndoChange", null, {
Multiple what? Multiple tabs? Was the initial intention to allow aggregation of undo actions from several tabs (one undo in bld, one in workshop, one in space, etc.), without allowing to chain undo actions of same type (several crafts)?
-
repo owner @Ethaniel_Kavelaars A single logical unit of operation should go into a single undochage if possible. Undo changes can and probably should be chained, but jury is still debating on how big margin of error is allowed for a player. Even 1 maximum undo with override is OK at the current state IMO.
-
repo owner - changed status to resolved
Fix
#150Triggering of the “undo” button in case of building purchase has been added in c7e45e5d4caeede4604d8cc439e625713ee2a17b but no implementation of the undo method has ever been done, so the button should not be triggered as it does nothing
→ <<cset d02d0e8b0eae>>
-
repo owner Merged in Ethaniel_Kavelaars/kittens-game/UNDO_BUILDING (pull request #443)
Fix
#150→ <<cset 122f1bd25f54>>
-
@bloodrizer Argh, I missed your last week comment, sorry!
Concerning how to manage undo:
O.K., I understand that allowing to undo only the very last action is already a big enough help for players, and I agree with that; Actually I use only F5, so I didn't even realize that the “undo” button didn't work for building purchase before reading the current issue report! But, in that case, there is a lot of useless/YAGNI code which blurs the actual intention, better simplify the code in order to make crystal clear for future readers what is allowed and what will not (I'll do the code cleaning tomorrow I think).
Concerning what to manage with undo:
Currently only workshop crafting can be undone, here are the other things that could be undone (if I don't miss some):
- stackable purchase (buildings) => I think that no undo must be provided, as it's already possible to sell buildings1
- one-time purchase (technologies/upgrades) => I have no opinion at all, I see no reason to allow undo for that, but I see no reason to not allow undo, so maybe I'll implement that some day and the jury will merge or reject the PR
- conversion (trade/sacrifice/refine/combust) => as it's very similar to workshop crafting (converting some resources into other ones), I think that those conversions should benefit too from undo for consistency, except maybe TC combustion2; I'll see how to code that in the next few days
-
repo owner Undo for buildings is fine, it's time limited anyway. I would probably allow undo anywhere where it makes sense from QOL point of view.
-
Thanks, noted!
- Log in to comment
I confirm that “undo”: 1/ does nothing for Bonfire building purchase (no refund, no removal of the building: If intended, then the button should not appear), 2/ does not exist for Religion and Space buildings (maybe it should if 1/ is a bug? ping @bloodrizer), 3/ works pretty fine for resource crafting, BUT it does not refund consumed resources that was initially above its storage cap (so it's not a real undo in that case).
BTW on a loosely related note, in the Religion tab, Order of the Sun upgrades can be sold, but neither Ziggurats nor Cryptotheology buildings, there is an inconsistency here.