-
-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an undo button. #29
Comments
Hey drwhut, I think I might be able to work on this. I just wanted to run somethings by you first. I was thinking of making a getter function in Piece("GetSleeping"). Every so often linear search through _pieces etc. to see if they are all sleeping and if so, then use get_state to add the state to a (pythonic) list. This list would be operated like a stack and when the user pressed the undo button the -1th(python shortcut for last address in a list) will be then set with set_state. I did have some questions about where(_physics_process?) the check would be done and how often(every 5 seconds? 10? constantly?) the check would be performed. I was thinking that this check/feature would be added to room.gd and the list/stack would be a new var added to the top of room.gd. Let me know what you think about this solution and if I need to clarify some things. I haven't done any code yet so I could run this by you first. |
Hey! In terms of your proposed implementation, here's a couple of pointers that might help you:
There are also pages in the documentation about how to do pull requests and the coding guidelines if you need them. I've also been thinking about this issue a lot (it's been open for a while, haha) - while this method will work in most cases, I think I've identified a few drawbacks to this method:
So we can either implement the original idea (what you plan to implement) and figure out these problems later in another issue, or we could try and figure out a better method for figuring out when to create these undo states. What would you prefer to do? |
I thought of the first issue you mentioned as well, my thought was that once it saved a state then maybe it would wait 30 seconds/some amount of time before checking again but this also has problems. The other two problems you mentioned are not easily addressed and will only become nastier as this game gets larger and people start making their own games with potentially MANY assets. I would prefer to try and come up with a better way to create these undo states. One idea I had is to make it like a more aggressive autosave where maybe every ten seconds/some amount of time we save the current state(whether all/some/none pieces are sleeping) and we just have a fixed number of saved states in the stack/list and maybe treat it more like a ringed buffer where the newest states will replace the oldest states. It would be nice if there was an easy and efficient way to measure the memory size of a state. note: maybe, since we can get how many pieces there are, we can "pretend" we can measure the memory usage and use that. (ie. maybe over 100 pieces counts as more "spaces" and so we end up saving less states) |
There are already autosaves in the game (save files are made every 5 minutes by default, although the interval can be changed in the options menu), so having the undo states be generated every so often like you said would just be the same functionality as the autosave, except they are saved in memory, not on the disk. One idea I had is that we create an undo state whenever an "action" is done, for example, just before a new object is spawned in, or when the room table is changed, or when the table is flipped. That way, we're guaranteed that a state is made only when something changes, and it avoids the rolling ball issue. I like the idea of using the number of pieces as a gauge of how much memory it's using, the only potential problem with that idea though is that containers can have multiple different objects inside of them, so it may be the case that even though there is one container on the table, there could potentially be thousands of objects inside of that container (it's unlikely that will ever be the case, but it could still happen). I've had a quick look online, and there doesn't seem to be a way of getting the size of objects in bytes. There is a way of getting the total memory used by the game (which is actually displayed when you press |
Ok, I like that idea. That way only useful states would be grabbed as you mentioned and it serves a purpose unique to the autosave feature. The problem you mentioned with the containers is a valid point, is there a way to check the containers for children? If we're saving the state less often(not on a timed interval) then we might be have time to check the containers for children. We should compile a list of all events upon which we would want this undo system to save the state. note: So basically autosave would be used if, say, someone knocked over all the pieces but undo would be used if someone removed/created a piece by accident. |
There is a function I think instead of making a list of events, maybe we could just create a function that creates an undo state on the host machine, and call that function wherever it makes sense in I've just realised as well we should also have a way to stop undo states being "spammed", e.g. if someone keeps adding objects to the room, then each object shouldn't make a new undo state, there should be some sort of timer that expires before a new undo state is made. Yeah, I think the way you put it in your note makes sense. |
Maybe we could put a timer on where the function is called in each event. ie. if you add an object it will take 10 seconds/some amount of time before it will again save the state when an object is added. You gave me good information about using Godot Arrays as stacks and I'd be able to use get_state and set_state to heavily base that part of my code on, setting a timer or something similar(maybe once you save when a piece is added, you wait out 5 pieces before saving when one is added again) shouldn't be too bad since there's already things like it in the code. The only thing I'm not too sure about is how I would make only the host have the stack. |
Yeah, having thought about it, I think there should be a timer for each event. There's no way to specifically have only hosts run certain functions, it's more of a convention thing for this project:
|
Could I use Godot timers or do you want me to use variables(after saving after new piece, wait 5 more pieces before saving again)? Outside of this I think I can do this, set up a Godot Array for a stack and call the function to add to it in certain events and the calls have some kind of timer on them. |
I haven't implemented the timers yet, but I have implemented a basic undo system, but I encountered a problem. When a piece is added I push an undo state, and when I press the undo button I pop an undo state, I used print statements to make sure everything was getting called the way I thought it was and I was right. set_state(state) hasn't been working for me, my code says a state was set but all the same cards are on the board(I tested it with the default asset pack playing cards). Do you know what could be wrong here? Is this supposed to be the behaviour of set_state(state)? |
I've used variables up until this point, so I'd say variables just for the sake of consistency. Can you send screenshots or a video of what's going wrong with |
So I set up custom functions to handle the pushing and popping of the states and I have an undo button. A state is added when a piece is added. When I test it everything is called when it should be, but when I press the undo button and it says that the pop func is called none of the pieces disappear like they should if the state was really being set(I was using the playing cards to test). Here are the images of my code: |
Sorry for the late reply! I think I've figured out what the problem is - if a function is set as either This also means that you'll have to have the pop function be remotely callable by other players, to be executed on the host. Here's how I would imagine it working from the button being pressed to the undo happening:
|
I tried out using the rpc_id(1, ...) to send the request to the host and setting the state in an rpc (rpc("set_state", state)) but I encountered some problems. The first problem was that I'm testing this in single player and so rpc_id(1,...) gave me errors. The other problem was that when I replaced the rpc_id's with regular rpc's nothing happened and the functions weren't called(the states were set when the pieces were added but were not called after that when they should have). |
What errors is |
Thank you, after adding the "master" keyword before my two functions now things work! I did have to add something so that add_piece doesn't add a state when it's called as a part of set_state but it now works. I still have to implement the timer(adding a piece will only add a state again after 5 pieces), make it for the other events we mentioned and cleanup the code to the coding standards, but at least it's functional now. |
Hey, something I noticed is that add_piece is also called in another circumstance, I was hoping you could tell me a little bit about it. When I add two pieces and don't move them, it counts that I have added two pieces. If I then move the piece on top to reveal the piece on the bottom it counts as me having added a piece. This may be intended but I just wanted to check. My description might be confusing but I could try to send you a video or screenshots. |
If the pieces are either cards or tokens, then yes this is the intended functionality. When two or more cards are put on top of one another, they become one object called a stack. |
Ok yes I was testing with the playing cards, thanks for clearing this up. |
Is your feature request related to a problem? Please describe.
Right now, if the player makes a mistake (e.g. they knocked over objects they didn't mean to), then unless the player made a save prior to the mistake then there is no way for the player to undo the mistake.
Describe the solution you'd like
Add an undo button that, when pressed, loads back a recent, previous state.
Describe alternatives you've considered
This could also be implemented using autosaves, but I decided that this method, while it would be more complex to implement, would end up being the simplest method for the player to revert the state since it just requires a button press rather than loading a save file.
Additional context
This could be implemented using a stack of states, where a state is added to the stack when no pieces are moving (i.e. they are all "sleeping").
The text was updated successfully, but these errors were encountered: