-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Foam and Conveyor Belts now process as fast as possible, Adds the Fast Process subsystem #16357
Conversation
…t Process subsystem
if(thing) | ||
thing.process(wait) | ||
else | ||
SSobj.processing.Remove(thing) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MrStonedOne input?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically .Remove() is a proc call. but proc call overhead is different for byond procs, I haven't actually profiled it, I just use -= out of fear of proc call overhead
neato |
name = "Fast Process" | ||
priority = -1 | ||
wait = 1 | ||
dynamic_wait = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MrStonedOne can a subsystem have dwait AND MC_TICK_CHECK
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in this case dwait will mainly just mean that the subsystem will not make up missed ticks, if it fires 2ds late, its next tick will still be 7ds away, not 5ds.
how ever he sat dwait lower to 1ds, that should be... interesting to test out.
Overall 👍 |
seems a bit too fast for foam. doesn't create the slip hell it was balanced around. |
for(var/atom/movable/A in affecting) | ||
if(!A.anchored) | ||
if(A.loc == src.loc) // prevents the object from being affected if it's not currently here. | ||
step(A,movedir) | ||
items_moved++ | ||
if(items_moved >= 10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly a question - not sure if this is really a problem (maybe you already know or it's fine), but with removed limit i see a constant CPU usage - https://dl.dropboxusercontent.com/u/73967379/A344DDH3dqqPP9--i/dasd32sada.png - 1000(!) PDAs trying to move into the wall, which makes high CPU usage (local server with no players). But yeah, 1k atoms... Probably not an issue for normal rounds and only one scenario in my mind: someone blocks conveyor belt and then drops huge amount of items until it starts to eat alot of CPU and slowing down everything else or no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh, this should be fine since 1k items arent gonna be ona single belt so
👎 don't make this default for stuff man |
@@ -12,7 +12,7 @@ | |||
var/amount = 3 | |||
animate_movement = 0 | |||
var/metal = 0 | |||
var/lifetime = 6 | |||
var/lifetime = 40 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Iamgoofball
This renders the foam's reagent reactions basically useless. In order to avoid duplicating reagents, the amount that reacts during each processing cycle is the total volume divided by the total lifetime, so that the sum of all the reactions that happen is equal to the volume put in the foam.
With a lifetime of 40, it means that instead of having 6 reaction of Volume/6 , we now have 40 reaction of Volume/40.
The issue is that the foam reagent reactions use the VAPOR method which check for clothes protecting the mob's skin. reaction_mob() is currently using a threshold (https://github.com/Iamgoofball/-tg-station/blob/68e752b46153e32d116d7a0d916de5c7c1ca199c/code/modules/reagents/chemistry/reagents.dm#L40) because even bio suits don't give a 100% protection so we need the threshold to not have small reagent amounts still reacting with the mob wearing a bio suit.
Now that you divide the reaction amount by 40 instead of 6, it means that any foam reaction with less than 20u of a given reagent won't have any effect, even on naked humans (and you probably need 40+ unit for people with just jumpsuits and shoes)
Another effect is that reagent reaction that gives a message to the mob will now spam the mob with 40 messages in a very short time.(styptic powder or condensed capsaicin foam for example, or worse a strange_reagent foam would spam every observer with 40 messages for every suicided human in the foam)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
goddamn it bawhoppen
Please revert the part about foam. |
@phil235 Nah, I'll just redo foam logic so it's compatible with this system. |
6 months later... |
What broke |
I'll redo it when oranges redos slip code |
No, you don't. |
you're not allowed to tell me I have to "fix" something without telling me what's broken you cuntlord and are you certain it's not a result of your damage refactor |
Do you not see the 3 paragraph long comment I made on March 30th, and to which you even responded? |
DEMO: https://www.dropbox.com/s/0jlzml802kv7emj/foam2.mp4?dl=0
Adds the Fast Process subsystem. Basically fastmos for when we want to process objects, like foam!
Foam, Conveyor Belts, and Mining Machines now run on this. Ore loads will be done as fast as the game can handle it, and foam will spread at its original speed pre-chemfoam.