Fix element list updates #21

Vectornaut · 2024-11-16T02:58:41Z

Vectornaut commented

2024-11-16 02:58:41 +00:00

The incoming branch address issue #20 by making the following changes:

In the application state, we store a serial number associated with the current assembly, which is incremented every time we load a new assembly.
We include that serial number in the diff key of each element outline item. On the main branch, the diff key is already unique within each assembly, so adding the assembly serial number makes the diff key unique throughout the session (until the serial number wraps around).

If we switch to a system where multiple assemblies can be loaded at the same time, we can use the storage key of the current assembly in place of the serial number of the current assembly.

The incoming branch address issue #20 by making the following changes: - In the application state, we store a serial number associated with the current assembly, which is incremented every time we load a new assembly. - We include that serial number in the diff key of each element outline item. On the main branch, the diff key is already unique within each assembly, so adding the assembly serial number makes the diff key unique throughout the session (until the serial number wraps around). If we switch to a system where multiple assemblies can be loaded at the same time, we can use the storage key of the current assembly in place of the serial number of the current assembly.

Vectornaut added 1 commit 2024-11-16 02:58:42 +00:00

Key element outline items by assembly 8daef5f71e

Introduce an assembly serial number, which increments when we load a new
assembly. Include it in each element outline item's diff key.

glen commented

2024-11-16 03:44:57 +00:00

If we switch to a system where multiple assemblies can be loaded at the same time, we can use the storage key of the current assembly in place of the serial number of the current assembly.

If you'd prefer, you could just have each Assembly grab a serial number from the state when it is created (updating the serial number in the process). Then you will get a unique key for each entity in each assembly, no matter how dyna3 may be refactored. (You might have an objection that a serial number isn't part of a platonic universe, but even universes need addresses. The serial number won't interact with the contents of the universe in any way, so it's an engineering convenience, not a problem.)

Anyhow let me know if you'd rather change to that scheme, or have me review the code as is. Should be quick on my end either way, there's not too much to this PR.

> If we switch to a system where multiple assemblies can be loaded at the same time, we can use the storage key of the current assembly in place of the serial number of the current assembly. If you'd prefer, you could just have each Assembly grab a serial number from the state when it is created (updating the serial number in the process). Then you will get a unique key for each entity in each assembly, no matter how dyna3 may be refactored. (You might have an objection that a serial number isn't part of a platonic universe, but even universes need addresses. The serial number won't interact with the contents of the universe in any way, so it's an engineering convenience, not a problem.) Anyhow let me know if you'd rather change to that scheme, or have me review the code as is. Should be quick on my end either way, there's not too much to this PR.

Vectornaut commented

2024-11-18 19:36:26 +00:00

If you'd prefer, you could just have each Assembly grab a serial number from the state when it is created

That would simplify the code a little, and it might have a slight advantage over using storage keys to identify assemblies in the future: we wouldn't have to worry about storage keys getting re-used. ~~I'll switch to that system.~~ Edited: actually, I'd prefer not to switch, for the reasons discussed below.

> If you'd prefer, you could just have each Assembly grab a serial number from the state when it is created That would simplify the code a little, and it might have a slight advantage over using storage keys to identify assemblies in the future: we wouldn't have to worry about storage keys getting re-used. ~~I'll switch to that system.~~ *Edited: actually, I'd prefer not to switch, for the reasons discussed [below](https://code.studioinfinity.org/glen/dyna3/pulls/21#issuecomment-1914).*

Vectornaut commented

2024-11-18 19:45:50 +00:00

Whoops, I just remembered why I didn't store the serial number in the assembly. Right now, for simplicity, the application state just contains one fixed Assembly object. There's no code for replacing that Assembly with a new one. The example-building routines work by removing all the elements in the assembly and then adding new elements.

I did this because adding code to replace the Assembly would require changes throughout the program, which felt outside the scope of this PR. It's likely that those changes would have to be reconsidered when we design a proper system for loading assemblies.

In light of that, please review the code as it is.

Whoops, I just remembered why I didn't store the serial number in the assembly. Right now, for simplicity, the application state just contains one fixed `Assembly` object. There's no code for replacing that `Assembly` with a new one. The example-building routines work by removing all the elements in the assembly and then adding new elements. I did this because adding code to replace the `Assembly` would require changes throughout the program, which felt outside the scope of this PR. It's likely that those changes would have to be reconsidered when we design a proper system for loading assemblies. In light of that, please review the code as it is.

Vectornaut added 1 commit 2024-11-18 19:50:06 +00:00

Start assembly serial number at zero b479e57446

Vectornaut commented

2024-11-19 00:16:23 +00:00

As we discussed in our meeting today, this PR has been superseded by #22. I'm keeping the outline-update-fix branch in case we decide later that we'd rather use this alternate approach.

As we discussed in our meeting today, this PR has been superseded by #22. I'm keeping the `outline-update-fix` branch in case we decide later that we'd rather use this alternate approach.

Vectornaut closed this pull request

2024-11-19 00:16:30 +00:00