Slimefun4 Cumulative Armor-Scheduling changes (Rainbow Armor + Radiation update)

Cumulative Armor-Scheduling changes (Rainbow Armor + Radiation update)

Open TheBusyBiscuit opened this issue 3 years ago • 10 comments

This is a cumulative pull requests consisting of:

[x] #3059
[x] #3131

Aug 01 '21 12:08 TheBusyBiscuit

Your Pull Request was automatically labelled as: "🎈 Feature" Thank you for contributing to this project! ❤️

Aug 01 '21 12:08 github-actions[bot]

After the new radiation system was added, armor-update-interval (which is in seconds) causes delayed application of the hazmat suit, which would result in you getting up to 10 seconds without it applying. Should this delay be reduced, and if done how? We have four options:

Not do it at all
Do it, and ask users to change it down to one/two
Do it, and convert armor-update-interval to ticks which would give a 0.5s update interval, along with radiation-update-interval also being converted to ticks and being set to 20.
Do a config update converting 10 to 1, somehow.

Aug 01 '21 12:08 Semisol

After the new radiation system was added, armor-update-interval (which is in seconds) causes delayed application of the hazmat suit, which would result in you getting up to 10 seconds without it applying. Should this delay be reduced, and if done how? We have four options:

Not do it at all

Do it, and ask users to change it down to one/two

Do it, and convert armor-update-interval to ticks which would give a 0.5s update interval, along with radiation-update-interval also being converted to ticks and being set to 20.

Do a config update converting 10 to 1, somehow.

I am not quite sure I understand what you mean

Aug 01 '21 13:08 TheBusyBiscuit

I am not quite sure I understand what you mean

Essentially after the update this was how radiation applied (1 line = 1 second):

radiation !
radiation !
radiation !
radiation ! 
radiation !
radiation ! 
radiation !
radiation !
radiation !
radiation armortask
...

and, if you wear a hazmat suit, it won't take effect until the next armortask

Aug 01 '21 13:08 Semisol

I see, perhaps the "armor update" should be moved to the super class then, we could implement a "minimum delay" to reduce redundant runs then. In other words, every one of the "armor" tasks triggers an update to the cached armor but updates will be skipped if the last update has been less than x ticks ago.

Aug 01 '21 13:08 TheBusyBiscuit

I see, perhaps the "armor update" should be moved to the super class then, we could implement a "minimum delay" to reduce redundant runs then. In other words, every one of the "armor" tasks triggers an update to the cached armor but updates will be skipped if the last update has been less than x ticks ago.

Looks good to me. I'll PR in my other changes.

Aug 01 '21 19:08 Semisol

Kudos, SonarCloud Quality Gate passed!