Firelock Doors Now Respond Correctly When Trying To Open With No Power #26472
Conversation
ShadowCommander
added
the
Status: Awaiting Changes
|
Cheers for the review! Looks a lot better now 🤘 |
|
Failure looks to be the same as the one in master caused by the disposal tests so I'll reopen this for review. |
github-actions
bot
added
Status: Needs Review
| /// </summary> | ||
| [DataField, AutoNetworkedField] | ||
| public bool Powered; | ||
|
|
||
| /// <summary> |
There was a problem hiding this comment.
I don't really like adding even more Powered fields to components as these will just need ripping out later, it should really be on some separate shared component so multiple systems can take advantage of it.
There was a problem hiding this comment.
I'm not sure how I feel about this. Are we really making things simpler by doing this? We're taking a boolean flag to be replicated and changing it into a full fat component. All we're doing right now is listening for an event and setting a flag. Now we'd be listening for an event and adding a component. That seems like we're doing more work to achieve the same thing. Maybe it allows us to merge the APIs...but I'm skeptical to that. I suppose it would be some sort of powered system that listens for a Power change event and adds a powered component to an object? Possibly...I would be interested to know if that would cover all existing cases, and if it's saving that much of a headache.
Regardless, I'm still new to this codebase so if it's believed to be the best course of action I'm happy to make the change of course! Just wary of over engineering something.
There was a problem hiding this comment.
Although, thinking about the idea for debug that powering something could be as easy as adding a component...that actually sounds pretty useful. So now I'm talking myself into it a bit more 😆
|
Hey, firelocks are still improperly predicted with this. Firelocks can be powered but locked due to pressure/temperature around them. Since the client doesn't have that information it still mispredicts when the firelock is powered but holding off a spacing for example. |
|
D'oh, that's a really good point thanks! Somehow forgot about that... With all the useful feedback I've gotten I'm going to close this for now and reopen when I've thought about the approach a bit more. Thanks everyone 👍 |
If you want to draw from something, I had made a PR that made some general improvements to firelocks before I knew about your pr. I have drafted that pr awaiting yours to be merged (or now your future pr). Because I didn't know your pr existed I did also fix prediction for firelocks in it, if you want to take anything from that feel free. You can probably improve it too. |
|
@Huxellberger Hey are you still working on a new pr? |
|
Not at the moment sorry! Busy with a few other matters but happy for someone to take this as inspiration. |
About the PR
Code changes to the firelock system to enable client prediction of powered status.
Why / Balance
Fixes #25975 and #25788
Technical details
Previously there was no way for the client to know the firelock was unpowered. This meant we would have nothing in our way to calling
SharedDoorSystem::StartOpeningand predicatively trying to open the door.I've added a new boolean value to the firelock component simply called
powered- all it does is cache the powered status for the firelock, which is the missing information we need to properly respond when trying to open the door barehanded without power.After making this change I noticed the lack of any feedback was a bit confusing, so I added an additional message to be displayed to the user so they know the reason their open action failed was the absence of power.
Media
firelock_fix.mov
Breaking changes
Changelog
🆑 Jake Huxell