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.

Sign up for GitHub

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

Jump to bottom

Redefinitions do not utilize CAN_BE_REDEFINED #2056

Closed
Drulikar opened this issue Dec 20, 2024 · 1 comment · Fixed by #2058
Closed

Redefinitions do not utilize CAN_BE_REDEFINED #2056

Drulikar opened this issue Dec 20, 2024 · 1 comment · Fixed by #2058
Assignees
@Cyberboss
Labels
Area: DMAPI Communication between TGS and DM Feature Request Requested functionality Ready Issue ready to be addressed
Milestone
v6.13.0

Comments

@Drulikar
Copy link

Describe the bug
I am opting to turn on spacemandmm's redefined_proc to be a warning but currently core.dm and

tgstation-server/src/DMAPI/tgs/core/tgs_version.dm

Lines 22 to 27 in f3bfa22

/datum/tgs_version/Wildcard()
return minor == null || patch == null
/datum/tgs_version/Equals(datum/tgs_version/other_version)
if(!istype(other_version))
return FALSE
fail this check:

image

To Reproduce
Steps to reproduce the behavior:

  1. Add redefined_proc = "warning" under [diagnostics] in SpacemanDMM.toml
  2. Compile with two above files present in a ss13 repository
  3. Observe warnings due to lack of CAN_BE_REDEFINED(TRUE) usage (or just not redefining)

Expected behavior
Ability to turn on redefined_proc linting and TGS dm files to pass.

Logs
Not applicable.

Server State: (please complete the following information):

  • OS: Linux
  • Version: TGS_DMAPI_VERSION "7.3.0"
  • Database Type/Version: N/A
  • BYOND Version Used: 515.1647
  • git Repository Used: https://github.com/cmss13-devs/cmss13
  • Origin Commit hash Used: N/A
  • Active Test Merges: N/A
  • Client Version: N/A

Additional context
Obviously I can edit the files myself, but that complicates an Automatic TGS DMAPI Update. Making a PR to add CAN_BE_REDEFINED(TRUE) to these procs then forces everyone using TGS to have an up to date spacemandmm.dm definition file that specifies CAN_BE_REDEFINED(X). So then I'm not sure if you would rather not redefine procs or what.
@Drulikar Drulikar added Bug Something's fucky Reproduction Required Reproduction steps required for issue labels Dec 20, 2024
@Drulikar Drulikar mentioned this issue Dec 20, 2024
Merged
@Cyberboss Cyberboss added Feature Request Requested functionality Area: DMAPI Communication between TGS and DM and removed Bug Something's fucky Reproduction Required Reproduction steps required for issue labels Dec 21, 2024
@Cyberboss Cyberboss added this to the v6.13.0 milestone Dec 21, 2024
@Cyberboss Cyberboss added the Ready Issue ready to be addressed label Dec 21, 2024
Cyberboss added a commit that referenced this issue Dec 21, 2024
@Cyberboss
Compatibility with SpacemanDMM's redefined_proc lint
f8dc0d3 
Closes #2056
@Cyberboss Cyberboss mentioned this issue Dec 21, 2024
Merged
@Drulikar
Copy link
Author

Thank you o/

github-merge-queue bot pushed a commit to cmss13-devs/cmss13 that referenced this issue Jan 2, 2025
@Drulikar
Proc Redefinition Linting (#7862)
20d6b31 
# 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
<details>
<summary>Screenshots & Videos</summary>

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

</details>


# Changelog
:cl: 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:
github-merge-queue bot pushed a commit to cmss13-devs/cmss13 that referenced this issue Jan 2, 2025
@Drulikar
Proc Redefinition Linting (#7862)
cc9b4de 
# 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
<details>
<summary>Screenshots & Videos</summary>

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

</details>


# Changelog
:cl: 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:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Cyberboss Cyberboss

Labels
Area: DMAPI Communication between TGS and DM Feature Request Requested functionality Ready Issue ready to be addressed
Projects
None yet
Milestone
v6.13.0
Development

Successfully merging a pull request may close this issue.

Compatibility with SpacemanDMM's redefined_proc lint tgstation/tgstation-server
2 participants
@Cyberboss @Drulikar