Add an undo button. about tabletop-club HOT 19 CLOSED

Ok yes I was testing with the playing cards, thanks for clearing this up.

from tabletop-club.

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.

from tabletop-club.

Hey! In terms of your proposed implementation, here's a couple of pointers that might help you:

• Piece inherits from RigidBody, which already has a property called sleeping, so you don't need to make a new getter function for it.
• GDScript arrays have functions for accessing them like a stack: push_back, pop_back, push_front, pop_front. I don't think GDScript has negative addressing like Python does.
• It makes sense to put the 'checking' loop in physics_process, since you're checking a physics property.
• I'd say check every 10 seconds, but have this be a constant at the top of Room.gd so it can easily be changed.
• Only the host should need to store and load these states, it'd be redundant if the other players did the same thing.
• Also make sure to have an upper limit on the number of states stored on the stack - it would probably make sense to limit it by memory usage (what if the state has a lot of pieces in?) rather by length, but if it's not possible to check the memory usage of the stack, then by length will be fine for now.

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:

• If the state of the table never changes, then it will just keep adding that state to the stack when ideally, it should only add one instance of that state to the stack (so the player can undo further back if they wanted to) - to check for this would mean adding more code to the linear search loop.
• What if there is a rolling ball on the table? In that case, a state will never be added to the stack unless it stops rolling (since it's not sleeping)!
• The linear search loop every 10 seconds would be problematic when the state has a lot of pieces in.

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?

from tabletop-club.

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)

from tabletop-club.

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 F3 in-game), but this is only available in Debug builds, not Release ones. So I think for now we can just limit the stack by the number of states, and we can always adjust it or improve the system later on.

from tabletop-club.

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.
-object spawned in, object removed, table changed, table flipped, maybe when/before a player leaves if any assets leave with them etc.

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.

from tabletop-club.

There is a function PieceContainer.get_piece_count(), but it only counts the direct children, not sub-children (e.g. a container within a container).

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 Room.gd (e.g. at the start of add_piece). If we miss any instances, we can always just add them afterwards by calling the function.

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.

from tabletop-club.

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.

from tabletop-club.

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:

• Variables and functions that should only be used by the host should be prefixed with srv_, e.g. _srv_undo_stack.
• If a function needs to be RPC'd, it should use the master prefix.

from tabletop-club.

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.

from tabletop-club.

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)?

from tabletop-club.

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 set_state? What you're trying to do is indeed the intended functionality.

from tabletop-club.

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:
set_state problem images.zip

from tabletop-club.

Sorry for the late reply! I think I've figured out what the problem is - if a function is set as either puppet or remotesync, then as a security measure I get that function to check if it was called remotely by the host. In your code, you call it directly, so instead try to RPC set_state to all clients from the host.

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:

• The button is pressed, and an RPC is sent from the client to the host, asking to pop an undo state.
• The host then pops the undo state, and sends the RPC to all clients (including themselves) to set the state that was just poped off the stack.

from tabletop-club.

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).

from tabletop-club.

What errors is rpc_id(1,...) giving you? Is it because the function you're trying to call hasn't been set as master?

from tabletop-club.

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.

from tabletop-club.

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.

from tabletop-club.

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.

from tabletop-club.