Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: SHOULD_RESPECT_PROC_SIGNATURE #409

Open
FluffyGhoster opened this issue Sep 16, 2024 · 1 comment
Open

Feature Request: SHOULD_RESPECT_PROC_SIGNATURE #409

FluffyGhoster opened this issue Sep 16, 2024 · 1 comment

Comments

@FluffyGhoster
Copy link

FluffyGhoster commented Sep 16, 2024

Basically what we discussed in Coderbus:

It's possible to create awful and confusing scenarios using the proc inheritance and changing the arguments type of the proc, eg:

atom/proc/mything(mob/A)

atom/movable/mything(mob/living/A) <- this should error out, type is different

area/mything(mob/B) <- this should (optionally) error out, var name is different

turf/mything(mob/A, mob/living/B) <- ok, it's only extending

turf/simulated/mything(mob/A, mob/B) <- this should (optionally) error out, type is different

It would be useful to have a per-proc option to set, which follows eg. this rules:

--- If SHOULD_RESPECT_PROC_SIGNATURE(LAZY): ---

  • All inheritances should use args of the same type or a supertype (mob/living -> mob is ok, the other way it's not)
  • The signature of the proc should either match, be empty or extend with additional arguments after the parent ones are respected

--- If SHOULD_RESPECT_PROC_SIGNATURE(STRICT): ---

  • All of SHOULD_RESPECT_PROC_SIGNATURE(LAZY)
  • All inheritances should use the same type of argument
  • All inheritances should use the same name for the argument (this makes it more bearable to read the proc, than names keeping changing between parent and childs)
  • The signature of the proc should either match or extend with additional arguments the parent one

Optionally, there should be a pragma/setting to enforce it codebase-wide, ontop of the per-proc option (which is more transitional)

@PowerfulBacon
Copy link

PowerfulBacon commented Sep 29, 2024

+1, I would want this enforced codebase-wise, though I would not allow extending a procs signature with more variables.

/datum/proc/test(mob/a) should fail when /datum/extension/test(mob/a, atom/b) is encountered since it adds space for errors. Or the logic should be way more complex, and it should check to make sure that if you are using test(a, b) then you are calling it on a /datum/extension and not a variable typed with /datum

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

No branches or pull requests

2 participants