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

Remove and condense unnecessary side-overrides in core code #4678

Merged

Conversation

out-of-phaze
Copy link
Member

@out-of-phaze out-of-phaze commented Dec 27, 2024

Description of changes

Removes or condenses a lot of unnecessary side-overrides in core code.
Some of these commits are going to be sent to stable/staging as applicable because they're fixes rather than code quality edits. Done. #4679

Why and what will this PR improve

Found a lot of bugs/bad copypasta and also just improves code quality overall. Simplifies control flow and somewhat improves performance on hot procs like update_icon.

Authorship

Me

@out-of-phaze out-of-phaze added the ready for review This PR is ready for review and merge. label Dec 27, 2024
@@ -719,11 +719,8 @@ Turf and target are seperate in case you want to teleport some distance from a t
/obj/item/weldingtool/can_puncture()
return 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should return isOn() I suspect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking, just a note.

@out-of-phaze out-of-phaze force-pushed the codequality/side-override-audit branch from 9223dba to 67ce8b8 Compare December 27, 2024 03:56
@@ -378,7 +378,7 @@
return ..(user, distance, "", jointext(desc_comp, "<br/>"))

/obj/item/check_mousedrop_adjacency(var/atom/over, var/mob/user)
. = (loc == user && istype(over, /obj/screen/inventory)) || ..()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think /inventory might be correct, there's no need to drag and drop over other elements that I can think of.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/obj/item/organ/internal/augment/active/cyberbrain/handle_mouse_drop(atom/over, mob/user, params)
	if(!istype(over, /obj/screen))
		attack_self(user)
		return TRUE
	. = ..()

dunno, but it's not the only place that checks for specifically screen as opposed to inventory. was just going for what current code did, since by checking both with || we're defaulting to the less-specific one

MistakeNot4892
MistakeNot4892 previously approved these changes Dec 27, 2024
Copy link
Contributor

@MistakeNot4892 MistakeNot4892 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments are not blocking.

@MistakeNot4892 MistakeNot4892 merged commit b0d6aac into NebulaSS13:dev Dec 28, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR is ready for review and merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants