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

Proc Redefinition Linting #7862

Merged
merged 8 commits into from
Jan 2, 2025
Merged

Conversation

Drulikar
Copy link
Contributor

@Drulikar Drulikar commented Dec 20, 2024

About the pull request

This PR turns on redefinition warnings (which should fail lints since we Werror). This PR needs testing though as there are going to be actual changes from this refactor. One aspect for example I'm not sure about is whether an area power_change should just always update_icon (vendor code mistakenly was causing this). I have opted to just include that in the proc now and removed redundant calls. Another couple notable example are that repair_robotic_organs now has an implementation for repeat_step_criteria and pkd_special revolvers that aren't a l_series subtype will no longer have burst changes.

TGS DMAPI also fails this, but I am waiting advice on what to do about this, so for now it will serve just as a test to ensure lints fail. See tgstation/tgstation-server#2056 (This is now fixed)

Explain why it's good for the game

In almost all cases, a redefinition is not necessary. And in the few cases it is, CAN_BE_REDEFINED(TRUE) can be used for the proc. See code changes for examples of all the obvious mistakes allowing redefinitions has caused.

Testing Photographs and Procedure

Screenshots & Videos

Put screenshots and videos here with an empty line between the screenshots and the <details> tags.

Changelog

🆑 Drathek
balance: The PKD special burst values have changed for non-"Double" (l_series) variants
code: Code no longer uses proc redefinitions with a few exceptions for global macros
fix: Fixed various things like all backpacks (instead of just JIMA flags) processing
/:cl:

@Drulikar Drulikar added Testmerge Candidate we'll test this while you're asleep and the server has 10 players Needs Testing Need to test it on the guinea pigs (production server) labels Dec 20, 2024
@Drulikar Drulikar requested a review from fira as a code owner December 20, 2024 09:09
@cmss13-ci cmss13-ci bot added Balance You need to be a professional veteran game maintainer to comprehend what is being done here. Code Improvement Make the code longer Fix Fix one bug, make ten more labels Dec 20, 2024
@cm13-github cm13-github added the Merge Conflict PR can't be merged because it touched too much code label Dec 27, 2024
@cm13-github
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

# Conflicts:
#	code/game/machinery/colony_floodlights.dm
#	code/game/machinery/vending/cm_vending.dm
@cm13-github
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@cm13-github cm13-github removed the Merge Conflict PR can't be merged because it touched too much code label Dec 28, 2024
realforest2001 pushed a commit to realforest2001/forest-cm13 that referenced this pull request Jan 1, 2025
cm13-github added a commit that referenced this pull request Jan 2, 2025
cm13-github added a commit that referenced this pull request Jan 2, 2025
cm13-github added a commit that referenced this pull request Jan 2, 2025
cm13-github added a commit that referenced this pull request Jan 2, 2025
cm13-github added a commit that referenced this pull request Jan 2, 2025
@harryob harryob added this pull request to the merge queue Jan 2, 2025
cm13-github added a commit that referenced this pull request Jan 2, 2025
Merged via the queue into cmss13-devs:master with commit cc9b4de Jan 2, 2025
28 checks passed
cmss13-ci bot added a commit that referenced this pull request Jan 2, 2025
@Drulikar Drulikar deleted the Redefinition_Warning branch January 2, 2025 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Balance You need to be a professional veteran game maintainer to comprehend what is being done here. Code Improvement Make the code longer Fix Fix one bug, make ten more Needs Testing Need to test it on the guinea pigs (production server) Testmerge Candidate we'll test this while you're asleep and the server has 10 players
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants