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

Fixes wrong signal usages #10875

Merged
merged 7 commits into from
Apr 24, 2024
Merged

Conversation

EvilDragonfiend
Copy link
Member

@EvilDragonfiend EvilDragonfiend commented Apr 10, 2024

About The Pull Request

Fixes wrong signal usages

Regex search to find issues

  • NOTE: Match Case option is necessary
  • RegisterSignals\([a-zA-Z_]+, COMSIG : searches for signals with wrong usage - RegisterSignals should take list
    Changes into RegisterSignal
  • RegisterSignals\([a-zA-Z_]+, list\(COMSIG[a-zA-Z_]+\) : searches for a signal with a single list
    Changes into RegisterSignal
  • RegisterSignal\([a-zA-Z_]+, list : searches for signals with wrong usage - It can be a list or
    Changes into RegisterSignals
  • RegisterSignal([s]?)\([a-zA-Z_]+, [a-z_]+ : searches for signals that might have been taken variable
    Changes into RegisterSignals
  • RegisterSignal([s]?)\([a-zA-Z,/\s_]+\/proc : searches for non-macro procs for signals
    Changes into TYPE_PROC_REF()

Why It's Good For The Game

Bug fix, runtime fix, etc

Testing Photographs and Procedure

This isn't what I can't test fully

Changelog

🆑
code: fixed wrong usage of signal procs
/:cl:

Comment on lines 21 to 22
RegisterSignalsDynamic(parent, change_on, PROC_REF(handle_change))
RegisterSignalsDynamic(parent, remove_on, PROC_REF(handle_removal))
Copy link
Member Author

Choose a reason for hiding this comment

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

If you wonder how TG managed that, here you are:

	// change on can be a list of signals
	if(islist(change_on))
		RegisterSignals(parent, change_on, PROC_REF(handle_change))
	else if(!isnull(change_on))
		RegisterSignal(parent, change_on, PROC_REF(handle_change))
	// remove on can be a list of signals
	if(islist(remove_on))
		RegisterSignals(parent, remove_on, PROC_REF(handle_removal))
	else if(!isnull(remove_on))
		RegisterSignal(parent, remove_on, PROC_REF(handle_removal))

Copy link
Member

Choose a reason for hiding this comment

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

I would rather cover this edge case than add a DEFINE that's used twice and will be quickly forgotten about.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed

Copy link
Member

@PowerfulBacon PowerfulBacon left a comment

Choose a reason for hiding this comment

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

Good catches

code/datums/components/COMPONENT_TEMPLATE.md Outdated Show resolved Hide resolved
code/datums/components/COMPONENT_TEMPLATE.md Outdated Show resolved Hide resolved
Copy link

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

@PowerfulBacon PowerfulBacon added this pull request to the merge queue Apr 24, 2024
Merged via the queue into BeeStation:master with commit d38e7d7 Apr 24, 2024
13 checks passed
DrDuckedGoose pushed a commit to DrDuckedGoose/BeeStation-Hornet that referenced this pull request May 11, 2024
* Signal fixes

* Removes dynamic macro

* I didn't save this.. Removes dynamic macro.

* Update COMPONENT_TEMPLATE.md

* nitpick

* nitpick 2
DrDuckedGoose pushed a commit to DrDuckedGoose/BeeStation-Hornet that referenced this pull request May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants