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 error if a proc has both should_not_sleep and set waitfor = 0 #211

Merged
merged 1 commit into from
Sep 9, 2020

Conversation

MrStonedOne
Copy link
Contributor

See tgstation/tgstation#53553 (comment) for why this logic is invalid.

@optimumtact
Copy link

optimumtact commented Sep 9, 2020

There should be a different lint define for this case, rather than allowing set waitfor when people expect no sleeping.

We are not the only people using this so we can't change the API contract like that.

@MrStonedOne
Copy link
Contributor Author

But set waitfor doesn't sleep!

It just guarantees the caller will not be locked up, the same thing should_not_sleep tries, but fails to guarantee.

the only time set waitfor = false would kick in, is in a sleep that should_not_sleep doesn't catch.

And because byond is not a statically typed language, should_not_sleep can not catch every case where something can sleep, trying to is as mathematically impossible as the halting problem, you can get very close, but not perfect detection.

It makes no sense to force disabling of set waitfor = false to use should_not_sleep, that seems like the kind of thing where, "We are not the only people using this", so we shouldn't be telling them if they can or can not use both at once.

Copy link
Owner

@SpaceManiac SpaceManiac left a comment

Choose a reason for hiding this comment

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

should_not_sleep and set waitfor = FALSE are static and dynamic ways to (attempt to) accomplish the same goal of not blocking the caller. They are not in conflict and warning on their combination is unnecessary.

@SpaceManiac SpaceManiac changed the title Fix spacemandmm erroring if a proc had both should_not_sleep and set waitfor Remove error if a proc had both should_not_sleep and set waitfor = 0 Sep 9, 2020
@SpaceManiac SpaceManiac changed the title Remove error if a proc had both should_not_sleep and set waitfor = 0 Remove error if a proc has both should_not_sleep and set waitfor = 0 Sep 9, 2020
@SpaceManiac SpaceManiac merged commit b7390e2 into SpaceManiac:master Sep 9, 2020
@SpaceManiac SpaceManiac added this to the suite v1.6 milestone Sep 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants