spacebuild / sbep

Spacebuild Enhancement Pack.
Apache License 2.0
75 stars 89 forks source link

Elev system needs a sanity check. #95

Closed Shigbeard closed 8 years ago

Shigbeard commented 8 years ago

So the elevator system needs a sanity check. ENT:GetFloorNum()... to be precise. When it returns 0, self.FloorTable[self:GetFloorNum()] returns nil. This variable is used in math.Round, which requires a number and will error on a nil value. By assuming floor 0 is floor 1, you can avoid this issue.

https://github.com/Shigbeard/sbep/tree/elev_system-sanitypatch I made a patch, but if I make a pull request it will break git blame, as it thinks I've rewritten the whole file because I'm shit at git.

Shigbeard commented 8 years ago

Sorry about the double post, i accidentally pressed enter without entering anything. XD

nirahiel commented 8 years ago

We already have a branch where we are working on the elevator system. I think x-coder can see your changes and do something about it. I can't. Japan expo for 4 days. Le 7 juil. 2016 05:45, "Shigbeard" notifications@github.com a écrit :

Ooops, let me edit OP.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/spacebuild/sbep/issues/95#issuecomment-230971499, or mute the thread https://github.com/notifications/unsubscribe/AAs2hGsC-cxlRGUilXak6fEMpCdX55Qeks5qTHY9gaJpZM4JGtii .

Shigbeard commented 8 years ago

Well the change i've made has allowed me to use the elevators now as opposed to having them simply spam errors so I'll use my changes until an official patch is made to master.

X-Coder commented 8 years ago

Thank you, I will look into it. By the way, the trouble you had with git is because you somehow changed all the line endings, but I can do the commit for you too.

X-Coder commented 8 years ago

@Shigbeard can you describe what steps you did to show up this error?

I can't see how you would get this error in first place: self.TargetOffset in line 189 is never nil because its initialized with -60.45 and only changed to some other offset if self.CallFloorTable[1] is not nil.

self.CallFloorTable is a table of all queued floor numbers, if no floor was called it has a default entry for first floor, so this table is never empty or nil. The "if self.CallFloorTable[1] then" in line 185 prevents a invalid call for self:GetFloorNum() which would then cause to set a invalid nil value for self.TargetOffset leading to your math.Round error in next line.

Shigbeard commented 8 years ago

Spawn a lift with any number of floors. Wire the elevator in such a way that floornum will be 0 until a floor is selected for the first time. I have some E2 that will serve this purpose

EDIT: In my testing, self.CallFloorTable[1] would return 0 if you had just spawned it. Here is exactly how I got the error.

I would spawn a lift system. Lets say it had 7 floors. No errors yet. I spawn an E2 chip, which renders an EGP screen for selecting floors (functioning by setting FloorNum to an integer matching the floor number). Initial value for this E2 is 0. When wired to the elevator, Server Console will begin spouting errors for line 188. Debugging this line showed me that self.TargetOffset was nil. line 185 SHOULD stop this, however this fails to account for a case where the value isn't nil but is infact 0.

When I run ENT:GetFloorNum() on the elevator when in this error state, I get a return value of 0. Unfortunately, this doesn't also equate to false. When I run ENT.FloorTable[0] I get a return value of nil. When I run math.Round( nil ) I get an error.

My adjustment only addressed this one case where self.CallFloorTable[1] would return 0 when it should at least return 1.

X-Coder commented 8 years ago

This should fix your issue by preventing adding invalid range to CallFloorTable in first place.

Shigbeard commented 8 years ago

It certainly looks like it will. Thank you.