Skip to content

Commit

Permalink
Proc Redefinition Linting (#7862)
Browse files Browse the repository at this point in the history
# 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:
  • Loading branch information
Drulikar authored Jan 2, 2025
1 parent 1ff3a87 commit cc9b4de
Show file tree
Hide file tree
Showing 113 changed files with 378 additions and 740 deletions.
1 change: 1 addition & 0 deletions SpacemanDMM.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ engine = "auxtools"

[diagnostics]
var_in_proc_parameter = "error"
redefined_proc = "warning"
2 changes: 1 addition & 1 deletion code/__DEFINES/MC.dm
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@
NEW_SS_GLOBAL(SS##X);\
PreInit();\
}\
/datum/controller/subsystem/processing/##X/fire() {..() /*just so it shows up on the profiler*/} \
/datum/controller/subsystem/processing/##X/fire() {CAN_BE_REDEFINED(TRUE); ..() /*just so it shows up on the profiler*/} \
/datum/controller/subsystem/processing/##X

#define log_qdel(X) log_debug(X)
2 changes: 2 additions & 0 deletions code/__DEFINES/__spacemandmm.dm
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#define SHOULD_BE_PURE(X) set SpacemanDMM_should_be_pure = X
#define PRIVATE_PROC(X) set SpacemanDMM_private_proc = X
#define PROTECTED_PROC(X) set SpacemanDMM_protected_proc = X
#define CAN_BE_REDEFINED(X) set SpacemanDMM_can_be_redefined = X
#define VAR_FINAL var/SpacemanDMM_final
#define VAR_PRIVATE var/SpacemanDMM_private
#define VAR_PROTECTED var/SpacemanDMM_protected
Expand All @@ -23,6 +24,7 @@
#define SHOULD_BE_PURE(X)
#define PRIVATE_PROC(X)
#define PROTECTED_PROC(X)
#define CAN_BE_REDEFINED(X)
#define VAR_FINAL var
#define VAR_PRIVATE var
#define VAR_PROTECTED var
Expand Down
2 changes: 2 additions & 0 deletions code/__DEFINES/_globals.dm
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#ifndef TESTING
#define GLOBAL_PROTECT(X)\
/datum/controller/global_vars/InitGlobal##X(){\
CAN_BE_REDEFINED(TRUE);\
..();\
gvars_datum_protected_varlist[#X] = TRUE;\
}
Expand All @@ -44,6 +45,7 @@

#define GLOBAL_SORTED(X)\
/datum/controller/global_vars/InitGlobal##X(){\
CAN_BE_REDEFINED(TRUE);\
..();\
##X = sortAssoc(##X);\
}
Expand Down
50 changes: 0 additions & 50 deletions code/_onclick/ai.dm
Original file line number Diff line number Diff line change
@@ -1,43 +1,3 @@
/*
AI ClickOn()
Note currently ai is_mob_restrained() returns 0 in all cases,
therefore restrained code has been removed
The AI can double click to move the camera (this was already true but is cleaner),
or double click a mob to track them.
Note that AI have no need for the adjacency proc, and so this proc is a lot cleaner.
*/

/mob/click(atom/A, list/mods)
..()

if(!client || !client.remote_control)
return FALSE

if (mods["middle"])
A.AIMiddleClick(src)
return 1

if (mods["shift"])
A.AIShiftClick(src)
return 1

if (mods["alt"])
A.AIAltClick(src)
return 1

if (mods["ctrl"])
A.AICtrlClick(src)
return 1

if (world.time <= next_move)
return 1

A.attack_remote(src)
return 1

/*
AI has no need for the UnarmedAttack() and RangedAttack() procs,
because the AI code is not generic; attack_remote() is used instead.
Expand All @@ -49,16 +9,6 @@
/mob/living/silicon/ai/RangedAttack(atom/A)
A.attack_remote(src)

/mob/UnarmedAttack(atom/A)
if(!client || !client.remote_control)
return FALSE
A.attack_remote(src)

/mob/RangedAttack(atom/A)
if(!client || !client.remote_control)
return FALSE
A.attack_remote(src)

/atom/proc/attack_remote(mob/user as mob)
return

Expand Down
45 changes: 42 additions & 3 deletions code/_onclick/click.dm
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,43 @@
* mob/RangedAttack(atom,params) - used only ranged, only used for tk and laser eyes but could be changed
*/

/*
AI ClickOn()
Note currently ai is_mob_restrained() returns 0 in all cases,
therefore restrained code has been removed
The AI can double click to move the camera (this was already true but is cleaner),
or double click a mob to track them.
Note that AI have no need for the adjacency proc, and so this proc is a lot cleaner.
*/

/mob/proc/click(atom/A, list/mods)
return FALSE
if(!client || !client.remote_control)
return FALSE

if(mods["middle"])
A.AIMiddleClick(src)
return TRUE

if(mods["shift"])
A.AIShiftClick(src)
return TRUE

if(mods["alt"])
A.AIAltClick(src)
return TRUE

if(mods["ctrl"])
A.AICtrlClick(src)
return TRUE

if(world.time <= next_move)
return TRUE

A.attack_remote(src)
return TRUE

/atom/proc/clicked(mob/user, list/mods)
if (mods["shift"] && !mods["middle"])
Expand Down Expand Up @@ -230,7 +265,9 @@
in human click code to allow glove touches only at melee range.
*/
/mob/proc/UnarmedAttack(atom/A, proximity_flag, click_parameters)
return
if(!client || !client.remote_control)
return FALSE
A.attack_remote(src)

/*
Ranged unarmed attack:
Expand All @@ -241,7 +278,9 @@
animals lunging, etc.
*/
/mob/proc/RangedAttack(atom/A, params)
return
if(!client || !client.remote_control)
return FALSE
A.attack_remote(src)

/*
Restrained ClickOn
Expand Down
1 change: 1 addition & 0 deletions code/_onclick/click_hold.dm
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
/// The history of all atoms that were hovered over while the mouse was depressed
var/list/mouse_trace_history
var/list/lmb_last_mousedown_mods
var/datum/entity/clan_player/clan_info

/client/MouseDown(atom/A, turf/T, skin_ctl, params)
ignore_next_click = FALSE
Expand Down
2 changes: 1 addition & 1 deletion code/_onclick/item_attack.dm
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
dig_out_shrapnel(user)

// No comment
/atom/proc/attackby(obj/item/W, mob/living/user,list/mods)
/atom/proc/attackby(obj/item/W, mob/living/user, list/mods)
if(SEND_SIGNAL(src, COMSIG_PARENT_ATTACKBY, W, user, mods) & COMPONENT_NO_AFTERATTACK)
return TRUE
SEND_SIGNAL(user, COMSIG_MOB_PARENT_ATTACKBY, src, W)
Expand Down
4 changes: 1 addition & 3 deletions code/controllers/subsystem/nanoui.dm
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ SUBSYSTEM_DEF(nano)
var/list/currentrun = list()
var/datum/nanomanager/nanomanager

/datum/controller/subsystem/nano/New()
. = ..()

/datum/controller/subsystem/nano/PreInit()
nanomanager = new()

/datum/controller/subsystem/nano/stat_entry(msg)
Expand Down
2 changes: 1 addition & 1 deletion code/datums/diseases/advance/symptoms/weight.dm
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ Bonus
transmittable = -2
level = 4

/datum/symptom/weight_loss/Activate(datum/disease/advance/A)
/datum/symptom/weight_even/Activate(datum/disease/advance/A)
..()
if(prob(SYMPTOM_ACTIVATION_PROB))
var/mob/living/M = A.affected_mob
Expand Down
2 changes: 1 addition & 1 deletion code/datums/effects/mob_crit/crit.dm
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
duration = 30
flags = INF_DURATION | NO_PROCESS_ON_DEATH | DEL_ON_UNDEFIBBABLE

/datum/effects/pain/validate_atom(atom/A)
/datum/effects/crit/validate_atom(atom/A)
if(isobj(A))
return FALSE
. = ..()
2 changes: 1 addition & 1 deletion code/datums/emergency_calls/upp_commando.dm
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
/datum/emergency_call/upp_commando/low_threat
name = "UPP Commandos"

/datum/emergency_call/upp_commando/create_member(datum/mind/mind, turf/override_spawn_loc)
/datum/emergency_call/upp_commando/low_threat/create_member(datum/mind/mind, turf/override_spawn_loc)
var/turf/spawn_loc = override_spawn_loc ? override_spawn_loc : get_spawn_point()

if(!istype(spawn_loc))
Expand Down
19 changes: 19 additions & 0 deletions code/datums/entities/player.dm
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,8 @@ BSQL_PROTECT_DATUM(/datum/entity/player)
load_player_data_info(get_player_from_key(ckey))

/client/proc/load_player_data_info(datum/entity/player/player)
set waitfor = FALSE

if(ckey != player.ckey)
error("ALARM: MISMATCH. Loaded player data for client [ckey], player data ckey is [player.ckey], id: [player.id]")
player_data = player
Expand All @@ -524,6 +526,23 @@ BSQL_PROTECT_DATUM(/datum/entity/player)
record_login_triplet(player.ckey, address, computer_id)
player_data.sync()

if(isSenator(src))
add_verb(src, /client/proc/whitelist_panel)
if(isCouncil(src))
add_verb(src, /client/proc/other_records)

if(GLOB.RoleAuthority && check_whitelist_status(WHITELIST_PREDATOR))
clan_info = GET_CLAN_PLAYER(player.id)
clan_info.sync()

if(check_whitelist_status(WHITELIST_YAUTJA_LEADER))
clan_info.clan_rank = GLOB.clan_ranks_ordered[CLAN_RANK_ADMIN]
clan_info.permissions |= CLAN_PERMISSION_ALL
else
clan_info.permissions &= ~CLAN_PERMISSION_ADMIN_MANAGER // Only the leader can manage the ancients

clan_info.save()

/datum/entity/player/proc/check_ban(computer_id, address, is_telemetry)
. = list()

Expand Down
21 changes: 11 additions & 10 deletions code/game/area/Sulaco.dm
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,7 @@
icon_state = "away1"


/area/shuttle/drop2/Enter(atom/movable/O, atom/oldloc)
if(istype(O, /obj/structure/barricade))
return FALSE
return TRUE


/area/shuttle/drop2
//soundscape_playlist = list('sound/soundscape/drum1.ogg')
Expand All @@ -74,6 +71,11 @@
ceiling = CEILING_REINFORCED_METAL
base_lighting_alpha = 0

/area/shuttle/drop2/Enter(atom/movable/O, atom/oldloc)
if(istype(O, /obj/structure/barricade))
return FALSE
return TRUE

/area/shuttle/drop2/sulaco
name = "\improper Dropship Normandy"
icon_state = "shuttle"
Expand Down Expand Up @@ -114,12 +116,6 @@
name = "\improper Normandy Landing Zone"
icon_state = "away2"


/area/shuttle/drop2/Enter(atom/movable/O, atom/oldloc)
if(istype(O, /obj/structure/barricade))
return FALSE
return TRUE

/area/shuttle/drop3
//soundscape_playlist = list('sound/soundscape/drum1.ogg')
soundscape_interval = 30 //seconds
Expand All @@ -128,6 +124,11 @@
ceiling = CEILING_REINFORCED_METAL
base_lighting_alpha = 0

/area/shuttle/drop3/Enter(atom/movable/O, atom/oldloc)
if(istype(O, /obj/structure/barricade))
return FALSE
return TRUE

/area/shuttle/drop3/sulaco
name = "\improper Dropship Saipan"
icon_state = "shuttle"
Expand Down
2 changes: 1 addition & 1 deletion code/game/gamemodes/extended/extended.dm
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
var/next_research_allocation = 0
taskbar_icon = 'icons/taskbar/gml_colonyrp.png'

/datum/game_mode/announce()
/datum/game_mode/extended/announce()
to_world("<B>The current game mode is - Extended!</B>")

/datum/game_mode/extended/get_roles_list()
Expand Down
8 changes: 0 additions & 8 deletions code/game/jobs/whitelist.dm
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,6 @@
if(player.check_whitelist_status(WHITELIST_FAX_RESPONDER))
LAZYADD(., "responder")

/client/load_player_data_info(datum/entity/player/player)
. = ..()

if(isSenator(src))
add_verb(src, /client/proc/whitelist_panel)
if(isCouncil(src))
add_verb(src, /client/proc/other_records)

/client
var/datum/whitelist_panel/wl_panel

Expand Down
4 changes: 0 additions & 4 deletions code/game/machinery/OpTable.dm
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,6 @@
else if(ismob(A))
..()

/obj/structure/machinery/optable/power_change()
..()
update_icon()

/obj/structure/machinery/optable/update_icon()
if(inoperable())
icon_state = "table2-idle"
Expand Down
18 changes: 4 additions & 14 deletions code/game/machinery/air_alarm.dm
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,13 @@
var/apply_danger_level = 1
var/post_alert = 1



/obj/structure/machinery/alarm/Initialize(mapload, direction, building = 0)
. = ..()

set_frequency(frequency)
if (!master_is_operating())
elect_master()

if(building)
if(loc)
forceMove(loc)
Expand All @@ -133,7 +135,6 @@

first_run()


/obj/structure/machinery/alarm/proc/first_run()
alarm_area = get_area(src)
area_uid = alarm_area.uid
Expand All @@ -148,13 +149,6 @@
TLV["pressure"] = list(ONE_ATMOSPHERE*0.80,ONE_ATMOSPHERE*0.90,ONE_ATMOSPHERE*1.10,ONE_ATMOSPHERE*1.20) /* kpa */
TLV["temperature"] = list(T0C-26, T0C, T0C+40, T0C+66) // K


/obj/structure/machinery/alarm/Initialize()
. = ..()
set_frequency(frequency)
if (!master_is_operating())
elect_master()

/obj/structure/machinery/alarm/Destroy()
if(alarm_area.master_air_alarm == src)
alarm_area.master_air_alarm = null
Expand Down Expand Up @@ -1017,10 +1011,6 @@ table tr:first-child th:first-child { border: none;}

return ..()

/obj/structure/machinery/alarm/power_change()
..()
update_icon()

/obj/structure/machinery/alarm/get_examine_text(mob/user)
. = ..()
if (buildstage < 2)
Expand Down
Loading

0 comments on commit cc9b4de

Please sign in to comment.