-
Notifications
You must be signed in to change notification settings - Fork 34
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
Create SuitPort to simplify EVA ingress and egress #1182
Comments
12.23.2023 - change: revise moveThere() in ExitAirlock and EnterAirlock - change: revise walkToEVASpot() in Task - new: add walkToActivitySpot() in Task Relates to #1182
Did you notice by any chance that during egress, settlers' movement through spots is different ? Somehow after some recent commits regarding activity spots, settlers are no longer able to walk into any one of the 4 chambers (zone 2) during egress. They will don the suit and prebreathe in zone 1. When done, they would skip zone 2 and go very quickly into zone 3 and then zone 4 and leave the airlock. On the other hand, during ingress, settlers' movement through spots are all good. |
The transitionTo(), vacate(), occupy(), and moveThere() all look fine. It's just that during egress, settlers couldn't step into the chamber. May be it fails to claim the spot. If that's the case, how they can claim the spot during ingress ? Can you take a look ? |
Sure but I find the airlock code very complex. I would like to write unit tests for it |
I haven't done anything but just looking at the code it's just very complicated. But my guess would be it is something to do with the
But any change will have to cover Enter & Exit Airlock as well as both Vehicle & Building Airlock. I'm happy to have a look in the future but only if we redesign it. Mark this issue as 3.8.0 and assign it to me if you want to redesign it. |
Yes this is the culprit. The following code doesn't work: /**
* Checks if the person is in a particular zone.
*
* @param p the person
* @param zone the zone of interest
* @return a list of occupants inside the chamber
*/
@Override
public boolean isInZone(Person p, AirlockZone zone) {
...
else if (zone == 2) {
return p.getActivitySpot() != null;
}
...
I've just fixed this issue as follows and will commit in a minute. else if (zone == AirlockZone.ZONE_2) {
for (var i: eva.getActivitySpots()) {
if (!i.isEmpty() && i.getID() == p.getIdentifier()) {
// logger.fine(p, 30_000L, "possessed " + p.getActivitySpot().getSpotDescription() + " in zone 2.");
return true;
}
}
return false;
}
Done !
We may as well rework the entire EVA Airlock to simplify the movement. I'll copy and paste the Additional context section above and start a brand new ticket for us to design EVA Port building, instead of refactoring this EVA Airlock with inflexible, prone-to-error ingress and egress task phases. I can see we can have a few variants of this EVA Port. Each chamber in this EVA port will operate independently from other chambers, thus removing the need of syncing up what everyone is doing in a chamber. Say EVA Port A is just for one person. EVA Port B is just for two persons. EVA Port C is just for four persons, etc. |
12.24.2023 - new: add AirlockZone enum - new: add Airlock wiki button in BuildingPanelEVA - fix: correct isInZone() in BuildingAirlock Close to #1182
Ok, fine. I still think the Task should know what zone the Person is in.. Ae can sort it as part of the rework |
Ok. Sure. We can work out a solution. With EVA port, we probably won't need 5 zones. We'll see. One big existing problem with the current implementation of TaskPhase is that we have a set duration defined for the whole task. However, in case of ExitAirlock and EnterAirlock, each task phase needs to have its own duration of time. How do we elegantly handle that in a standard way ? The logic gets more complicated when we need it to stop at a task phase, waiting for the air to pump in (to pressurize) or pump out (depressurize), etc. So, if only one person goes through the all those task phases, at least it will not have to wait for another person to get passed a certain task phase. |
1.2.2024 - fix: revise EnterAirlock Relates to #1182
1.2.2024 - fix: revise EnterAirlock Relates to #1182
1.2.2024 - change: revise EnterAirlock - change: revise addTime() in Airlock Relates to #1182
I wonder if a better way to solve this is to apply #1180 and have all the locations of the airlock modelling as activity spots? Then the spots outside the doors can be reserved as part of the walking task. |
The way we currently defined the activity spots in buildings.xml seems to have limited their locations to be within the bound of a building. Do feel free to change how they work. |
Describe the situation
State the goals
Java classes involved
Screenshots
If applicable, add screenshots to help explain your problem.
Specifications
The SuitPort Concept
Reference
The text was updated successfully, but these errors were encountered: