Skip to content

feat: give every MF_ flag one home #536

Description

@MarcusKainth

Where it lands

Decode or execute in SQL (sqlcpu)

Native mode's simulation, native/src/sql/sim/, rather than the SQL CPU.
The form's list has no entry for it.

What you are trying to do, and what stops you

A review asked me to state MF_SOLID once instead of adding a second copy,
and the answer turned out to be that there is no once to state it in.

p_mobj.h's flags are declared per file across the simulation. attacks.rs,
enemy.rs, inter.rs, map.rs, missile.rs, mobj.rs, plane.rs,
player.rs, pspr.rs, setup.rs and shoot.rs each carry their own block,
and most of the flags appear in several of them. MF_SHOOTABLE is in most of
those files. MF_SOLID is in four, and mobj.rs, which owns the things the
flags describe, is not one of them.

The practical effect is that a reviewer cannot tell a new duplicate from the
convention. Mine was the convention, and it took reading every file to say so
with any confidence. The next person adding a flag to a file that does not yet
carry it faces the same question and has the same amount of reading to do.

Nothing is wrong today. Every copy is p_mobj.h's own value and a test would
catch one that was not, so this is legibility rather than correctness.
AGENTS.md and the writing-voice rules both say to state a fact once and cite
the file that owns it, and these flags are the largest place that does not
happen.

What it might look like

One module under sim/ holding every MF_ the simulation uses, each with the
p_mobj.h line it comes from, and the eleven blocks replaced by imports. Flags
used in one place only could stay where they are or move with the rest;
moving them all is easier to check, because then the rule is "every MF_ is
imported" rather than a judgement per flag.

Worth doing as its own change rather than bundled, since it touches most of
the simulation and moves no behaviour, and a diff that large is only reviewable
when it is the whole of what it does. A test that fails if a const MF_ is
declared outside that module would keep it from drifting back, and is the part
that makes the change stick.

Does this touch any of these?

  • None of them

Would you want to implement it?

Yes

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementA new feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions