[RFC] Handle the impossibility of building a route during walk#3332
[RFC] Handle the impossibility of building a route during walk#3332ARGAMX wants to merge 3 commits into
Conversation
ARGAMX
force-pushed
the
handle-impossibility-of-building-route-during-walk
branch
from
085afb9 to
8286498
Compare
tobylane
left a comment
tobylane
left a comment
There was a problem hiding this comment.
Picking up the doctor from any future action, eg desk or cabinet, crashes with the error
Error in buttonup handler:
.CorsixTH/CorsixTH/Lua/humanoid_actions/walk.lua:31: Invalid value for parameter 'x'
stack traceback:
[C]: in global 'assert'
CorsixTH/CorsixTH/Lua/humanoid_actions/walk.lua:31: in local 'constructor'
CorsixTH/CorsixTH/Lua/class.lua:74: in global 'WalkAction'
CorsixTH/CorsixTH/Lua/entities/humanoid.lua:873: in method 'walkTo'
CorsixTH/CorsixTH/Lua/rooms/gp.lua:55: in method 'doStaffUseCycle'
CorsixTH/CorsixTH/Lua/rooms/gp.lua:96: in method 'commandEnteringStaff'
CorsixTH/CorsixTH/Lua/room.lua:406: in method 'onHumanoidEnter'
CorsixTH/CorsixTH/Lua/humanoid_actions/pickup.lua:54: in local 'interrupt_handler'
CorsixTH/CorsixTH/Lua/entities/humanoid.lua:675: in method 'setNextAction'
CorsixTH/CorsixTH/Lua/dialogs/place_staff.lua:68: in method 'close'
CorsixTH/CorsixTH/Lua/dialogs/place_staff.lua:174: in method 'onMouseUp'
CorsixTH/CorsixTH/Lua/window.lua:1703: in field 'onMouseUp'
CorsixTH/CorsixTH/Lua/ui.lua:895: in function <CorsixTH/CorsixTH/Lua/ui.lua:882>
(...tail calls...)
| while queue[i] and queue[i].must_happen do | ||
| -- Skip over any actions which must happen, | ||
| -- except when adding PickupAction for stuck humanoid | ||
| while queue[i] and queue[i].must_happen and not pickup_stuck_humanoid do |
There was a problem hiding this comment.
Why is it structured this way? if not (class.. and ..stuck) then while loop end is clearer.
There was a problem hiding this comment.
Why is it structured this way?
if not (class.. and ..stuck) then while loop endis clearer.
Ok, I applied it. 👌
| self.is_leaving = false -- Whether the humanoid is leaving. | ||
| self.no_truncate = false -- If set, disable shortening the action. | ||
| self.uninterruptible = false -- If true, can not be interrupted/cancelled by pickup. | ||
| self.stuck = false -- If true, humanoid had stuck during executing this action. |
There was a problem hiding this comment.
| self.stuck = false -- If true, humanoid had stuck during executing this action. | |
| self.stuck = false -- If true, humanoid has become stuck during the execution of this action. |
I can't reproduce it. But without canceling the pickup. |
ARGAMX
force-pushed
the
handle-impossibility-of-building-route-during-walk
branch
from
5ea135c to
90c63dc
Compare
Needs deeper insight, can we have the and/or reproduction steps please. |
|
I had accidentally built the room slightly differently to the video in the linked issue. In this layout, it is possible to drop the doctor by the door inside the room, move the plant in line with the others as a wall, then attempt to drop the doctor on his current side of the plant wall, with access to the door but not the desk. 
Following the design of the video more closely as in the second image, I can't drop the doctor on his side of the plant wall, but if I press Esc there is a different crash as it can't put the doctor back where they were (in master it does the teleport as in the issue video). 
|
Thanks. I am still unable to reproduce the provided examples without enabling "COMPLETELY ALLOWED" mode. 
. 
. But this PR doesn't have goal to fix them all. Although the video in #3330 may be misleading because it has "COMPLETELY ALLOWED" enabled, I recorded it for a slightly different case and then was too lazy to record a new one more suitable for #3330. Therefore, if you're testing something that falls outside the scope of "Steps to Reproduce" in #3330, you're essentially testing other situations whose solution is simply not within the goals of this PR. The crash when pressing Esc would also be great if someone will fix it. I haven't set that goal yet. It happens because instead of having a list of objects and coordinates for a room, the room is re-searched them every time the humanoid needs to approach them. Firstly, it slows down the game. Secondly, it causes crashes like these. While I generally consider the scenario invalid because it requires "COMPLETELY ALLOWED" to be enabled, I still believe the game shouldn't crash when pressing Esc. But not as a result of this certain PR. |
I'd like to know more if Toby is saying this PR has introduced this bug or not |
|
Yes, this PR adds a bug in completely allowed mode. If it's out of scope to fix it now, note it for later. |
Can you please provide a detailed scenario that doesn't crash Lua in 0.69.2, but does crash Lua in this PR? I still don't understand what specific bug this PR adds. Otherwise, in my understanding, the point here is that this PR doesn't fix something that already was broken before. |
|
Currently I'm plan to postponing this, skip 0.70.0, targeting this to 0.71.0. But that doesn't mean this PR doesn't need an Approve. It still does. Currently it's essentially been suspended allegedly due to some "bug", but still no concrete case has been given related to that "bug". 😜 |
ARGAMX
changed the title
3 participants
Fixes #3330
Describe what the proposed change does