search

snozbot / fungus

An easy to use Unity 3D library for creating illustrated Interactive Fiction games and more.
MIT License
1.63k stars 291 forks source link

Retain Command.ItemId on Cut and Paste #969

Closed ongjinwen closed 1 year ago

ongjinwen commented 3 years ago

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?

stevehalliwell commented 3 years ago

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.

breadnone commented 3 years ago

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

ongjinwen commented 3 years ago

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.

stevehalliwell commented 3 years ago

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.

TheEmbracedOne commented 2 years ago

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