Not useful anymore, it can only fail if the GameSave passed in is nullptr, which this commit guards against.
Also make Simulation::Load and Simulation::Save take block-oriented positions and rects.
flood_water would occasionally corrupt pmap by moving the pmap entry at i's location even if the entry itself didn't refer to i. pmap updates are tricky, they are best handled by Simulation::move.
Saving blockair/blockairh is nice because RecalculateBlockAirMaps uses the sim rng, which means the sim rng would get advanced in Simulation::Load. Also rename RecalculateBlockAirMaps to ApproximateBlockAirMaps because that's what it is, an approximation, and it's needed only if there are no block air maps in the save.
Simulation::frameCount keeps track of frames elapsed since the beginning of the simulation, zeroed at clear_sim. It overflows when it reaches the 64-bit limit, which means anything that depends on it should either handle this, or not fail catastrophically. sandcolour (the only thing that depends on it as of now) is a good example of the latter: sandcolour has a periodicity of 360 frames, which means that there is one sandcolour period that is cut short by the overflow. This is not "handled" (the period is cut short, which is detectable by users) but is not catastrophic either (it's not a big deal, and it won't ever happen unless someone hacks the save).
Also restrict saves with determinism data to 98.0.
Achieved by adding a new element property called CarriesTypeIn, whose bits signal to save loading code which properties of particles of the element class in question carry element IDs. The bits in this property are numbered the same way as sim.FIELD_* constants for consistency. One would signal from Lua that a custom element carries element IDs in its tmp like this:
elem.property(id, "CarriesTypeIn", 2 ^ sim.FIELD_TMP)
"Carrying an element ID in a property" is to be interpreted as follows: the property is treated as a combination of a PMAPBITS-bit (so, currently 9-bit) unsigned integer lower part holding an element ID and a 32-PMAPBITS-bit (so, currently 23-bit) signed integer upper part holding whatever makes sense for the element. CONV, for example, uses this signed integer in its ctype as the extra "v" parameter for particle creation.
Some checks on particles, most importantly whether their element IDs refers to an enabled element, were done _before_ in-save element IDs are mapped to in-simulation element IDs. This resulted in some particles being removed if their IDs were unlucky enough.
More precisely, refactor the code responsible for routing these GameController events to the Lua side. The issue with the previous solution was it relied on preprocessor macros to switch between Lua-ful and Lua-less builds.
This restrict effects of paste-time de-stacking to positions under particles being pasted. If this is not done, particles beyond the paste area can be wrongfully killed, see #889.
Some operator[]s that know the size of the container they wrap like to assert(index >= 0 && index < size), which is bad for us because we sometimes use &container[size]. This is not undefined behaviour until that pointer is dereferenced, but certain operator[]s choose to ignore this fact and err on the side of caution. The solution is to use &container[0] + size instead of &container[size].
I took extra care to not mess up signedness in readOPS in ab600780d0, but apparently didn't do the same in readPSv.
Also fix a bound check that was broken since aac6b7258c. It's a good thing this was broken, because this allowed negative type values from broken signedness readPSv to get past and cause a crash later on, rather than just cause particles to disappear or something.
When PHOT fails to move (do_move or eval_move return "no move"), it looks for
a surface (a contour of boundaries, as reported by is_boundary) along its path
and reflects off (or refracts into, see below) it, using get_normal_interp to
find the point of incidence and get_normal to deduce the surface normal.
get_normal is given the point and angle of incidence, and attempts to traverse
the surface the point belongs to by running two "surface scout" processes.
These processes remember their own position and "heading", a subset of the
eight cardinal directions on the grid. They are initialized with the point of
incidence and a heading that includes all directions whose dot product with
the angle of incidence is non-negative (see direction_to_map). They then
perform a few iterations (SURF_RANGE).
In each iteration, the processes check all eight neighbours of the cell they
are on and select the first neighbouring cell they find that is both a
boundary (as reported by is_boundary) and that is within their heading. They
then move to this neighbouring cell and update their heading by discarding
directions that are not similar enough to (differ by more than 45 degrees
from) the one that took them where they are now (see find_next_boundary). If
they find no such neighbour, they stop.
Continuing the militaristic line of thinking introduced by the term "surface
scout", you can imagine the two processes as two paratroopers who arrive from
above, land on a horizontal surface, and one starts going left, while the
other starts going right. They initially expect the surface they land on to be
close to horizontal, but are also prepared for not too erratic changes in its
angle as they go. Changes too erratic (imagine a precipice) scare them and
force them to stop.
Once the processes finish, an imaginary line segment is drawn between the
cells they ended up on. If the line segment is long enough (estimated by j,
and compared against NORMAL_MIN_EST), get_normal returns a normal that is
perpendicular to it. If it is too short, get_normal gives up and returns
nothing (which results in the PHOT being killed).
This amounts to our paratroopers attempting to get the "lay of the land" by
walking away from where they landed and comparing where they end up. They also
know that if they are still relatively close to each other at the end of their
walk, their measurement is probably wrong and their mission should be aborted.
The bug this commit fixes is that get_normal returns bogus surface normals
when it encounters thin walls of particles, defined as walls exactly two
layers of particles thick. One-layer walls are not really walls, as movement
code allows particles to penetrate these, and three-layer and thicker walls
are too thick for the bug to manifest.
The bug manifests for two-layer walls because the "left" scout process is
drawn to the side of the wall opposite to the one with the point of incidence.
This is because scout processes check neighbours in a clockwise order, and
always select the first suitable neighbour they find. As particles on the
other side of the wall are both boundaries and are within the heading of the
processes, they also qualify as suitable neighbours, so whether a scout
process selects the correct side of the wall depends on the order in which
neighbours are checked.
Essentially, the paratroopers look at their immediate surroundings in a
clockwise order. The right paratrooper always finds the ground and knows where
to step. The left paratrooper finds the Upside Down from Stranger Things and
teleports there.
This bug also affects refraction into and out of thin walls, but since these
walls are thin, the path the PHOT takes inside them is rather short and the
incorrect angle of travel is difficult to see. Furthermore, upon exit, the
same normal deduction bug causes the PHOT to take a path whose angle is almost
identical to that of the path that took it to the wall, so much so that it is
also difficult to see over shorter distances.
The solution is to have the left scout process check neighbours in reverse
order, so that it prefers the right side of the wall over the wrong one. This
does not affect its behaviour when facing thicker walls, but fixes its
behaviour when facing two-layer walls.
The changes in this commit also make find_next_boundary interact with
is_blocking directly to detect a change between the blocking trait of
immediate neighbours. This makes more sense than relying on is_boundary
because find_next_boundary is meant to find a transition from non-blocking to
blocking neighbours within the current heading, rather than to find any
boundary particle. The difference is subtle but important.
... and everything built around them.
A GameSave would hold at least one but sometimes two representations of a save:
one serialized, and one "friendly", accessible for modification. Thus, a
GameSave would have three states:
- "Collapsed": only the serialized representation was present; this was the
initial state of GameSaves loaded from files;
- "Expanded With Data": both the serialized and the friendly representations
were present; this was the state of GameSaves loaded from files after a call
to Expand;
- "Expanded Without Data": only the friendly representation was present; this
was the initial state of GameSaves being prepared for being saved to files.
A GameSave would be able to go from Collapsed to Expanded With Data with a call
to Expand, and back with a call to Collapse. Of course, this latter transition
would discard any changes made to the friendly representation, for example with
Translate. A GameSave would however be unable to go from Expanded Without Data
to any other state; a call to Collapse in this state would have been a no-op.
There were two instances of Collapse being called, one in the GameSave
constructor taking the serialized representation, immediately after a call to
Expand, and another in SaveRenderer, which would Collapse a save "back down" if
it had originally been Collapsed. Now, consider that there reasons for
constructing a GameSave from the serialized representation are as follows:
- loading an online save at startup from the command line;
- loading a local save at startup from the command line;
- loading a local save when it is dropped into the window;
- loading a local save for placement of the most recently used stamp;
- loading a local save for stamp placement via Lua;
- loading an online save for preview generation while browsing;
- loading a local save in the stamp browser for thumbnail generation;
- loading a local save in the local save browser for thumbnail generation.
In some cases, the friendly representation is needed for thumbnail generation
by ThumbnailRendererTask. ThumbnailRendererTask operates on its own copy of the
GameSave, because it runs SaveRenderer on a thread different from the main one
and cannot be sure of the lifetime of the original GameSave. It destroys this
copy when it is done rendering, so the call SaveRenderer makes to Collapse is
pointless.
In all other cases, the friendly representation is needed immediately. In some
of these, SaveRenderer is used from the main thread, but since the friendly
representation of the GameSave will be needed for pasting anyway, the call
SaveRenderer makes to Collapse is pointless again.
So, Collapse goes away. This also means that it is pointless for GameSaves to
hold on to the serialized representation, since in all cases in which they have
access to it, the friendly representation is needed immediately, and with
Collapse gone, they will never need it again.
get_normal_interp is given a PHOT and scans a line section starting from the PHOT, extended in the direction of its velocity, to determine the surface normal of the surface that is reflecting the PHOT. It uses is_boundary to detect "boundaries" on this line section, defined as one "blocking" cell with at least one "non-blocking" non-diagonal neighbour. It never actually makes sure the position it passes to is_boundary is within the simulation area, so I assume is_boundary is expected to handle this correctly.
Plot twist: it does not. It delegates checking whether a cell is "blocking" (defined as something PHOT would normally fail to move into as per eval_move, but GLAS and BLGA are handled specially) to is_blocking. is_blocking returns true for cells beyond the simulation area (as eval_move returns "no move" for such cells). Once is_boundary sees that the cell it's been given is blocking, it then proceeds to check whether any of its non-diagonal neighbours might be non-blocking. In most cases, non-diagonal neighbours of an out-of-bounds cell are also out of bounds and are thus blocking, but if the cell is_boundary is given is in the innermost layer of out-of-bounds cells, just beyond the simulation area, then it has a neighbour that is in bounds, and that one may not block PHOT. The takeaway is that out of bounds cells can indeed be boundaries, as far as is_boundary is concerned.
This is a problem because get_normal_interp's line section can easily reach beyond the simulation area if the PHOT's velocity is high enough, and if it finds a boundary along this line section, it immediately stops looking and passes its position to photoelectric_effect, which then uses this potentially out-of-bounds position to index pmap. A position with y = -1 causes photoelectric_effect to read from the last few slots of parts data that it then interprets as pmap entries, which then may direct it to particles to spark that are beyond parts. This eventually crashes.
This commit doesn't fix is_boundary's definition of boundaries, but it stops get_normal_interp looking at cells beyond the simulation area.
The crash is difficult to reproduce because there have to be many particles in the simulation for the very last slots of parts to be in use, and for them to point to memory that isn't accessible. PHOT also has to survive a try_move beyond the simulation area first (otherwise the reflection code isn't even run), which requires it to start from EHOLE. I have no idea why this is so. Reproduce the out of bounds read in photoelectric_effect with
break Simulation.cpp:2934 if nx < 0
in gdb and executing the following Lua code:
sim.clearSim()
tpt.set_wallmap(55, 44, 12)
local i = sim.partCreate(-1, 223, 178, 31)
sim.partProperty(i, "vx", -300)
sim.partProperty(i, "vy", 0)
sim.framerender(1)
This fixes bugs like "type\0hello mom" being a property name sim.partProperty accepts and half-fixes bugs like text formatting codes making gfx.drawText exit prematurely.
Also clear SaveRenderer graphics cache along with the main Renderer's when needed, and revert to built-in element callbacks rather than nothing at all when assigning nil to a callback slot in Lua.
Rather than hacking velocity, do it directly through can_move. Add a special case to make it slowly float upwards, rather than immediate like most weight differences in TPT.
This is a follow up to this crash fix - 0ed8d0a0be
This may possibly fix other crashes users occasionally experience, especially in the case of high velocity particles with loop edge mode on. Even so, there are other bugs at play, as a crash here can't be triggered if pmap is in a correct state
Also add GameSave::PressureInTmp3 to check for elements with pressure memory and fix TUNG not sampling pressure on creation. This does not in itself fix#822 because tmp3 and tmp4 are still saved in 16 bits each, so full ctypes still don't fit in tmp3.
This fixes a crash with water equalization on, with loop mode enabled, when there are high velocity particles near the edges. This can be occasionally reproduced with id:2800901
The water was moved to a new position and pmap updated, but the movement code continued, assuming water was at its old position. pmap for the WATR's old position won't be cleared once it moves, leaving a stale entry. If a particle then looks up the water in that location and tries to swap positions, this can cause a crash at the end of try_move
