Fix RaycastFloor flags #1328
Fix RaycastFloor flags #1328
Conversation
|
I would suggest raycastFlags as opposed to frc, for both the member name and define names |
|
agree with the above. "raycast floor check wall" doesnt make much sense at all. if the first floor is meant to convey raycast down, then that sounds better |
|
|
||
| if (func_80839034(play, this, sp58, BGCHECK_SCENE)) { | ||
| //! @bug groundPoly's bgId is not guaranteed to be BGCHECK_SCENE |
There was a problem hiding this comment.
That's a very interesting bug, I think it means the surfacetype array would be sourced from the scene but indexed with the dynapoly's poly's index
I guess if this came up in vanilla we would know by now, but I wonder if with spawning the right actor in the right spot in the right scene you could for example exit the scene by walking on the right dynapoly poly, though the context of having to use a door is very restrictive (or not... SRM can boomerang doors right 4Head)
src/code/z_bgcheck.c
Outdated
| @@ -527,10 +560,10 @@ void StaticLookup_AddPoly(StaticLookup* lookup, CollisionContext* colCtx, Collis | |||
| * Locates the closest static poly directly underneath `pos`, starting at list `ssList` | |||
| * returns yIntersect of the closest poly, or `yIntersectMin` | |||
| * stores the pointer of the closest poly to `outPoly` | |||
| * if (flags & 1), ignore polys with a normal.y < 0 (from vertical walls to ceilings) | |||
| * if BGCHECK_GROUND_CHECK_ON, ignore polys with a normal.y < 0 (from vertical walls to ceilings) | |||
There was a problem hiding this comment.
| * if BGCHECK_GROUND_CHECK_ON, ignore polys with a normal.y < 0 (from vertical walls to ceilings) | |
| * if groundChk & BGCHECK_GROUND_CHECK_ON, ignore polys with a normal.y < 0 (from vertical walls to ceilings) |
There was a problem hiding this comment.
feels unnecessary since BGCHECK_GROUND_CHECK_ON on its own expresses itself as being a groundChk flag in the name
There was a problem hiding this comment.
Well the function has a gazillion arguments
There was a problem hiding this comment.
I dont think you necessarily need to show the anding operation in the comment, but it can say something like "if BGCHECK_GROUND_CHECK_ON is passed" or "if BGCHECK_GROUND_CHECK_ON is set" to make it read more like a sentence
but yea I do think it should be clear just by association of the name what its referring to.
But in that same vein, groundChk should just be groundCheck with the whole word, its only two characters and it will better match the define names
There was a problem hiding this comment.
it isn't 2 characters, it's 20, and it would introduce an inconsistent variable names with some abbreviated some not. Unless perhaps you want to eliminate the chk abbr altogether which in that case it would be about 262 characters in bgcheck alone.
There was a problem hiding this comment.
obviously i meant 2 characters per usage of the word... and I dont think the total amount of characters it adds is really relevant, when all that really matters is how long individual lines get
its already inconsistent with the enum name, so pick your posion. I think the abbreviating is not necessary and not helping to solve any particular problem, it would be fine as the whole word
There was a problem hiding this comment.
The bigger picture point for me and this PR is that chk is an established abbreviation in the code base at the moment, primarily in bgcheck, camera, Math3D, and inherited from event_chk_inf. If groundChk should just be groundCheck, then arguably all chks should be checks, but I'd rather avoid doing something like that in this PR since it falls outside of the scope of what I want to do with this PR.
Either way, groundChk and GROUND_CHECK are equivalent to myself at least.
* fix raycast floor flags * format.sh * Name flags * Rename RaycastFloor functions, clean up caller code, document z_bg_spot15_rrbox * change comment to prevent format wrap * change to "if BGCHECK_GROUND_CHECK_ON is set" * roman suggestions
Resolves a minor issue where afew RaycastFloor functions were tied to the wrong flags