Would you like to react to this message? Create an account in a few clicks or log in to continue.

You are not connected. Please login or register

R1886 patch: Create a new connector may damage undo.

2 posters

Go down  Message [Page 1 of 1]

royqh1979



Issue1
Undo states will be messed up by doing the following steps:
(1) Delete a component that connects to a node ( the node will be auto deleted).
(2) Click where the old is to start a new connection. A new node will be created there, a new connector connected to it, and wait to end connection.
(3) Cancle the connection by press ESC.

Then step 1 will become undoable.

Issue2
When restore a connector  creation, the created connector is searched by its id. This can fail if node operations are not recored in undo ( like in the previous steps).
It may cause the "null connected node got when undo" bug.

This patch tries to fix them.

Note:
- I use a stack to replace the m_cicuitChange counter. Which is used as both a counter and a store of comp state start positions. When cancel a circuit modify, it's info is used to restore the comp states.
Attachments
R1886 patch: Create a new connector may damage undo. Attachmentundo.zip
You don't have permission to download attachments.
(2 Kb) Downloaded 2 times

arcachofo

arcachofo

Thanks.
I tried a similar approach based in your solution.
I think this issue is solved at Rev 1889.
Also added Undo for Component Properties at Rev 1888.

royqh1979



arcachofo wrote:Thanks.
I tried a similar approach based in your solution.
I think this issue is solved at Rev 1889.
Also added Undo for Component Properties at Rev 1888.

It seems that R1890 still has some issues:

- It seems cancelCircuitModify()  should be named as cancelCircuitChange() to match beginCircuitChange()/endCircuitChange()?

- If the new connector is created by click on a component pin and cancelled, m_circState.size() is 0 and saveState() won't create a new undo state. Then undo() will incorrectly undo the last modification.

So we must check if a new undo state is created before undo.

I've tried to make a patch:
- In cancelCicuitModify(), Undo() won't be called unless a new undo state is really created.
- Fixes: drag to create a new component and cancel it by press ESC, will uncorrectly undo the last modification too. It may even crash simluide if there's no undo state.
- UnsaveState() is removed since it shouldn't be called.
Attachments
R1886 patch: Create a new connector may damage undo. Attachmentundo3.zip
You don't have permission to download attachments.
(2 Kb) Downloaded 1 times

arcachofo

arcachofo

It seems that R1890 still has some issues:

- It seems cancelCircuitModify() should be named as cancelCircuitChange() to match beginCircuitChange()/endCircuitChange()?

- If the new connector is created by click on a component pin and cancelled, m_circState.size() is 0 and saveState() won't create a new undo state. Then undo() will incorrectly undo the last modification.
Yes, you are right.
I also renamed "CircuitChange" to "UndoSep", it was too similar to "CircuitModify".
Solved at Rev 1891.

drag to create a new component and cancel it by press ESC, will uncorrectly undo the last modification too. It may even crash simluide if there's no undo state.
I don't see this one: drag to create a new component always adds an undo state.

Sponsored content



Back to top  Message [Page 1 of 1]

Permissions in this forum:
You cannot reply to topics in this forum