Add brigmedic handheld #103
Conversation
|
ready to review |
| protected override void UpdateState(BoundUserInterfaceState state) | ||
| { | ||
| base.UpdateState(state); | ||
| switch (state) |
There was a problem hiding this comment.
if(state is not CrewMonitoringState st) return; ?
There was a problem hiding this comment.
no need for that, original CMO handheld code also doesnt have this check
There was a problem hiding this comment.
Well, it's better not to change the original code to avoid conflicts, but it could have been simplified in a separate version.
The switch doesn't really make sense here; you could have made it a one-liner if instead of all this.
There was a problem hiding this comment.
Well, it's better not to change the original code to avoid conflicts, but it could have been simplified in a separate version. The switch doesn't really make sense here; you could have made it a one-liner if instead of all this.
maybe will change that in the future cuz i am going to keep messing with this code for one of upcoming prs
There was a problem hiding this comment.
In a good implementation, the old state should have been made virtual, inheriting from the brigmed version to avoid duplication, and only transmitting the necessary lists on the server.
Then, on the client side, nothing would need to be changed in the interface, and you could use the same one.
There was a problem hiding this comment.
In a good implementation, the old state should have been made virtual, inheriting from the brigmed version to avoid duplication, and only transmitting the necessary lists on the server.
Then, on the client side, nothing would need to be changed in the interface, and you could use the same one.
thx, will think about that
There was a problem hiding this comment.
Some logic on the client side just to avoid overloading the server with it.
But this solution actually creates unnecessary calculations on the server that could have been avoided,
and would slightly reduce the traffic.
Yes, it's a negligible difference that doesn't have a major impact, but technically, it's a more correct solution from this side of the issue.
There was a problem hiding this comment.
But this solution actually creates unnecessary calculations on the server that could have been avoided,
and would slightly reduce the traffic.
Yes, it's a negligible difference that doesn't have a major impact, but technically, it's a more correct solution from this side of the issue.
alright, alright, i ll fix that someday
There was a problem hiding this comment.
It's not necessary to fix it, I'm saying it's not important, I'm just pointing it out for the future.
.../Prototypes/_StarLight/Entities/Objects/Specific/Medical/handheld_brigmedic_crew_monitor.yml
Outdated
Show resolved
Hide resolved
Short description
Dumbed version of cmo handheld that shows only security department.
Why? I want to change how people play as brigmedic. Instead of occasionally helping security revive prisoners, the brigmedic's main responsibility will now be ensuring the entire security department stays alive.
This is one of upcoming prs:
Checks
Changelog
🆑 Miracula