fungus icon indicating copy to clipboard operation
fungus copied to clipboard

Retain Command.ItemId on Cut and Paste

Open ongjinwen opened this issue 3 years ago • 5 comments

Description

Do not assign new ItemId to pasted commands on Cut and Paste. This is causing issue for me as I use ItemId to manage saves in my project.

What is the current behavior?

Paste always assign a new ItemId to pasted commands.

What is the new behavior?

  • Pasted commands will retain ItemId
  • Cut and Paste will clear CopyCommandBuffer

ongjinwen avatar May 02 '21 17:05 ongjinwen

Can you clarify the use case here?

I'm also concerned that this will introduce duplicate commandIDs when cut and pasting commands occurs between 2 flowcharts. Which I think is probably, primary use case of cut and paste right now.

stevehalliwell avatar Jun 05 '21 22:06 stevehalliwell

Perhaps reconsidering this pr before merging. Currently, Fungus would sometimes/randomly having duplicates IDs when copying/pasting if-else commands and Fungus will only consider the 1st copy and completely ignore the other. This may make it worse.

No bug report for this yet, bcos it's super random/rare and very hard to reproduce, but people on Fungus' discord server has been randomly encountered this issue every now and then

breadnone avatar Jun 06 '21 07:06 breadnone

Can you clarify the use case here?

I'm also concerned that this will introduce duplicate commandIDs when cut and pasting commands occurs between 2 flowcharts. Which I think is probably, primary use case of cut and paste right now.

I see. I wasn't aware that commands could be copied between flowcharts, as I have only tested it within blocks of a flowchart. Let me revise my changes.

I wanted a save (and history) system that implicitly tracks commands instead of using save points. For this, I require a way to uniquely identify the command objects. I looked around the properties and it seems item ID fits my use case. However, the current cut-paste behaviour (basically a copy-delete-paste) always initialize a new ID to the new command. So I made it such that cut-paste will always retain the ID if I need to move the commands between blocks. Oh, and cut-paste should erase the command buffer too. :P

Perhaps reconsidering this pr before merging. Currently, Fungus would sometimes/randomly having duplicates IDs when copying/pasting if-else commands and Fungus will only consider the 1st copy and completely ignore the other. This may make it worse.

No bug report for this yet, bcos it's super random/rare and very hard to reproduce, but people on Fungus' discord server has been randomly encountered this issue every now and then

Noted, I see what I can find.

ongjinwen avatar Jun 08 '21 16:06 ongjinwen

OK, you want to be able to cut and paste commands without breaking existing custom save files. That's quite reasonable. I think it's only part of that solution, there are plenty of other things Fungus would let you do that would break a custom save file.

I would suggest that you only retain command ids only if the cut and paste is on the same flowchart. Never do this when moving between flowcharts.

If you wanted to go further. The ability to configure or toggle this behaviour behind a different state of operation. As this behaviour is only there to ensure integrity of saves in a released product. It seems a good idea to introduce a Fungus setting that indicates the game is already released and either limits, alters, or warns the user when it does something that might cause issue for existing players.

Note. I'm going to remove this from the current milestone as it doesn't relate to a bugfix.

stevehalliwell avatar Jun 10 '21 20:06 stevehalliwell

@ongjinwen Dont know if this issue is still relevant but you could also use the blockname + command index combo to do what you described above. CommandIndex is the index of the command in its parent block (ie first command is 0, then the next is 1, the next is 2 and so on). This could probably solve the issue of pasting perhaps, unless you figured it out already.

Hope this is helpful.

TheEmbracedOne avatar Nov 18 '21 23:11 TheEmbracedOne