Skip to content
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

Open
mokun opened this issue Dec 23, 2023 · 9 comments
Open

Create SuitPort to simplify EVA ingress and egress #1182

mokun opened this issue Dec 23, 2023 · 9 comments
Assignees
Milestone

Comments

@mokun
Copy link
Member

mokun commented Dec 23, 2023

Describe the situation

  • While there are only 4 activity spots defined in EVA Airlock as the EVA chamber locations, there are in fact numerous other spots that are defined in runtime, such as next to the interior door and those next to the exterior door.
  • Movement between these spots are complicated and delicate and prone to breakage.
  • The TaskPhase approach within EnterAirlock and ExitAirlock has been stretched to an extreme.

State the goals

  • Need a less conflicting approach that simplify and limit cases, especially when one or more settlers are stuck at one phase and unable to move to the next phase/zone.
  • Avoid the following by looking ahead prior to walking there :
01-Adir-01:628.777 (Warning) Walk [x2] : [Terminus] Rik Declercq - Could not exit EVA Airlock 3.
01-Adir-01:628.777 (Severe) EVAOperation [x2] : [Terminus] Rik Declercq - Cannot walk to outside site.
  • Reduce the onset of the following by accurately checking if an empty chamber is available:
01-Adir-01:628.777 (Info) EnterAirlock : [Terminus - EVA Airlock 3] Cynthia Sanchez - All chambers are occupied in EVA Airlock 3. Could not enter.
  • Avoid starting the EVA until all chambers are occupied or time the chamber to open at certain time:
01-Adir-01:623.472 (Info) ExitAirlock : [Heritage - EVA Airlock 2] Anna Russo - Can't egress. Other occupant(s) already pre-breathed a quarter of time. Current task: Digging Local Regolith.
01-Adir-01:623.472 (Info) EnterAirlock : [Tian Cheng - EVA Airlock 1] Hung Yu - Ready to depressurize the chamber.
01-Adir-01:623.472 (Info) EnterAirlock : [Terminus - EVA Airlock 3] Joshua Rivera - Cannot ingress. All chambers are occupied in EVA Airlock 3.

Java classes involved

  • BuildingAirlock, EnterAirlock, ExitAirlock, EVA, Airlock

Screenshots
If applicable, add screenshots to help explain your problem.

Specifications

  • pre 3.7.0 build 9117

The SuitPort Concept

  • We could model the idea of SuitPort that are semi-exposed to the outside air
  • SuitPort will be a one-person port and greatly simplifies the procedures for ingress or egress.
  • It cuts down the steps of having to go through various activity spots.
  • We could assume anyone wanting to go outside would use a pre-breather device to purge the O2 around 50 millisols prior to using SuitPort for egress. This way, no one has to waste extra time in an airlock to prebreathe during egress.
  • Each SuitPort could house numerous EVA suits.
  • These suits would be docked in a partially pressurized compartment of an airlock building. After the person has donned the suit, this compartment will be open up and be exposed to the outside air.
  • The other side of the SuitPort is a compartment that's fully pressurized at the settlement standard pressure.
  • The person would then climb out of the suit from the back and hoist himself up to come back into the airlock.
  • Then he would remove the inner pressure suit and change back into regular garment for settlement living.
  • The SuitPort should have visual indicator on the Settlement Map for players to tell if each EVA Suit is being taken for use. The SuitPort should show if it's being partially pressurized or fully pressurized or in transition.

Reference

  1. https://www.youtube.com/watch?v=BUTKnm1BehQ
  2. https://www.youtube.com/watch?v=NUqm_-0P7y8
  3. https://www.youtube.com/watch?v=-tDuwIu_b9g
  4. https://www.youtube.com/watch?v=HQA5Mfstf1s
mokun added a commit that referenced this issue Dec 23, 2023
12.23.2023

- change: revise moveThere() in ExitAirlock and EnterAirlock
- change: revise walkToEVASpot() in Task
- new: add walkToActivitySpot() in Task

Relates to #1182
@mokun
Copy link
Member Author

mokun commented Dec 24, 2023

@bevans2000

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.

@mokun
Copy link
Member Author

mokun commented Dec 24, 2023

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 ?

@mokun mokun added this to the 3.7.0 milestone Dec 24, 2023
@bevans2000
Copy link
Member

Sure but I find the airlock code very complex. I would like to write unit tests for it

@bevans2000
Copy link
Member

bevans2000 commented Dec 24, 2023

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 isInZone method. The big question is why is this needed? As moving through the Airlock is controlled by a Task; the current state should be explicitly tracked. I would want to simplify the logic:

  • Use the Task phases to identify where the Person is in the Airlock, e.g. pre-breathe defines it must be in the middle zone.
  • I would replace the zone numbers with an enum to make the code more readible
  • Convert all the various one positions to ActivitySpot; this mean the claim/release logic will handle freely up space as Person move through
  • Only the chamber activity spots are shown on the map and reported as Function ActivitySpots

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.

@mokun
Copy link
Member Author

mokun commented Dec 24, 2023

my guess would be it is something to do with the isInZone method

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;
    	}

I would replace the zone numbers with an enum to make the code more readible

Done !

Convert all the various one positions to ActivitySpot; this mean the claim/release logic will handle freely up space as Person move through

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.

mokun added a commit that referenced this issue Dec 24, 2023
12.24.2023

- new: add AirlockZone enum
- new: add Airlock wiki button in BuildingPanelEVA
- fix: correct isInZone() in BuildingAirlock

Close to #1182
@bevans2000
Copy link
Member

Ok, fine. I still think the Task should know what zone the Person is in.. Ae can sort it as part of the rework

@mokun mokun self-assigned this Dec 24, 2023
@mokun mokun added the bug label Dec 24, 2023
@mokun
Copy link
Member Author

mokun commented Dec 24, 2023

Task should know what zone the Person is in..

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.

@mokun mokun modified the milestones: 3.7.0, 3.8.0 Dec 24, 2023
mokun added a commit that referenced this issue Jan 2, 2024
1.2.2024

- fix: revise EnterAirlock

Relates to #1182
mokun added a commit that referenced this issue Jan 2, 2024
1.2.2024

- fix: revise EnterAirlock

Relates to #1182
mokun added a commit that referenced this issue Jan 3, 2024
1.2.2024

- change: revise EnterAirlock
- change: revise addTime() in Airlock

Relates to #1182
@mokun mokun changed the title Simplify moving from spots to spots during EVA Create EVAPort to simplify EVA ingress and egress Jan 17, 2024
@mokun mokun changed the title Create EVAPort to simplify EVA ingress and egress Create SuitPort to simplify EVA ingress and egress Jan 18, 2024
@mokun mokun pinned this issue Jan 18, 2024
@bevans2000
Copy link
Member

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.

@mokun
Copy link
Member Author

mokun commented Jan 28, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants