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 made it possible to get rid of two GameSave constructors.
Also clean up Client::LoadSaveFile, Client::ReadFile, and Client::WriteFile in the process, and remove unused SaveRenderer::Render
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
The underlying problem was that the spreading step in SimulateGOL would record activity concerning a cell to builtinGol even if said cell already housed a non-GOL particle. The culling step handles these records and purges them once it's done (thus builtinGol only ever has non-zero values inside SimulateGOL), except in this case, it saw the non-GOL particle and skipped the cell without purging the corresponding records. This would later let GOL spread seemingly out of nowhere.
LITH: show .tmp2 in HUD, remove outdated comment
SLCN: update description
ROCK: form under pressure from both normal LAVA and LAVA(STNE), as these are basically the same thing
