Skip to content

Match phys::System contact listener flags and rigid body teardown - #188

Open
RobbyV2 wants to merge 2 commits into
zeldaret:masterfrom
RobbyV2:phys-system-contact-listener-flags
Open

Match phys::System contact listener flags and rigid body teardown#188
RobbyV2 wants to merge 2 commits into
zeldaret:masterfrom
RobbyV2:phys-system-contact-listener-flags

Conversation

@RobbyV2

@RobbyV2 RobbyV2 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

isActorSystemIdle reads two separate flags out of the _61 padding, so I split it into _61, _62 and _63 and left them u8; 0x7101216cb4 loads _61 with ldrb / cmp #0 / cset ne, and clang emits no cset for a bool. Writing the getStatus comparison into the return there short-circuits into an early ret and comes out four bytes short of the target's branchless orr, so I kept it in a local.

I named 0x71012167ec setDisableSensorContactPointInfoNotifications after the field it writes on the Sensor listener.


This change is Reviewable

@Pistonight Pistonight left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pistonight made 3 comments.
Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on RobbyV2).


src/KingSystem/Physics/System/physContactListener.h line 45 at r1 (raw file):

    void unregisterCollisionWithBody(RigidBody* body);

    bool get90() const { return _90; }

do we have better names for these?


src/KingSystem/Physics/System/physSystem.h line 147 at r1 (raw file):

    u8 _28[0x60 - 0x28];
    bool mPaused;
    u8 _61;

based on usage these are probably bools as well


src/KingSystem/Physics/System/physSystem.cpp line 192 at r1 (raw file):

bool System::isActorSystemIdle() const {
    const bool flag = _62 || _61;

does inlining flag work and do we have better names for _61 and _62?
(It's a bit weird if the programmer made a separate variable, they don't write if (flag) return true; immediately after)

Maybe return _62 || _61 || !mgr || mgr->getStatus() != act::BaseProcMgr::Status::ProcessingActorJobs is one line

@RobbyV2

RobbyV2 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

do we have better names for these?

Renamed 0x90 to mMagneMassScalingEnabled; the getters and setters seem oriented around magnetic mass scaling. I kept 0x91, since its mixed layer/cast-filter use doesn't support one name yet.

based on usage these are probably bools as well

Changed to bools.

does inlining flag work and do we have better names for _61 and _62?

(It's a bit weird if the programmer made a separate variable, they don't write if (flag) return true; immediately after)

Maybe return _62 || _61 || !mgr || mgr->getStatus() != act::BaseProcMgr::Status::ProcessingActorJobs is one line

Inlining it changed the branch structure and function size. The exact form computes is_idle and actor_jobs_idle, then returns their OR. Bool storage seems common enough, though the names are unknown, so I kept _61 and _62.

@RobbyV2
RobbyV2 requested a review from Pistonight August 3, 2026 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants