- 
                Notifications
    
You must be signed in to change notification settings  - Fork 101
 
Oxygen 2 #517
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
base: main
Are you sure you want to change the base?
Oxygen 2 #517
Conversation
        
          
                src/main/java/dev/galacticraft/impl/internal/mixin/oxygen/LevelChunkMixin.java
          
            Show resolved
            Hide resolved
        
      | 
           Not sure if this was a feature already but does copper still oxidize in no atmosphere environments. specifically could you design it so a sealed room will let copper oxidize but in no atmosphere environments it will no longer oxidize  | 
    
IT ACTUALLY WORKS!!!1
I don't think allowing vacuums is worth it
implemented for the sealer, not yet implemented for the bubble distributor it's getting even more complicated...
feat. even more cursed custom data structures!
fix text input for target bubble size clear bubble distribution before removing block entity also utilize states when tracking subscribers
rationale: * simplifies implementation - no more funky bitset logic and data growth mechanisms * in my mind, there shouldn't be more than ~5 sealers/distributors working in a single chunk section in the worst case, so linear time lookup should be fast enough
move chunk oxygen accessor to public api
improve implementation explanations as well
c26d1ec    to
    a5b498c      
    Compare
  
            
          
                src/main/java/dev/galacticraft/api/accessor/GCBlockExtensions.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | 
               | 
          ||
| import java.util.Iterator; | ||
| 
               | 
          ||
| public interface LevelOxygenAccessorRO { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the RO stand for, out of curiosity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It stands for read-only. The interface is split so that LevelReader can implement the read-only parts. It could be renamed to LevelOxygenReader or something if that's easier to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that makes sense. Don't feel you have to rename it, but a comment saying read-only might be nice.
        
          
                src/main/java/dev/galacticraft/impl/internal/mixin/oxygen/ChunkSerializerMixin.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/main/java/dev/galacticraft/impl/internal/mixin/oxygen/ChunkSerializerMixin.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/main/java/dev/galacticraft/impl/internal/mixin/oxygen/LevelChunkSectionMixin.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/main/java/dev/galacticraft/impl/internal/mixin/oxygen/LevelChunkSectionMixin.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/main/java/dev/galacticraft/impl/internal/mixin/oxygen/extinguish/TorchBlockMixin.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/main/java/dev/galacticraft/impl/internal/mixin/oxygen/extinguish/WallTorchBlockMixin.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like MAX_SEALER_VOLUME is being used as the maximum number of blocks that can be sealed, regardless of how many sealers there are. I feel like this probably shouldn't be the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also seems to be the case that covering up an oxygen sealer/subdividing a sealed room does not update the space without a sealer anymore to be non-breathable.
Replaces the old state-inversion based oxygen system with a new "provider-based" system, and updates the oxygen sealer and oxygen bubble distributor implementations to match. This allows for precise point based oxygen checks rather than block position based checks.
Additionally adds the ability for oxygen sealers to quickly update their status based on broken/placed blocks:
Because of this, sealers now only recheck their seals when there is a block broken/placed, rather than on a set interval (the interval is only used for initial seals, to prevent constant rechecking of large unsealable areas).
User facing changes in action: https://www.youtube.com/watch?v=NoDF3qlwULY
General Implementation Notes
Consider "breathable" to be equivalent to "has atmosphere" or "has air" for simplicity, as we don't currently support anything more than that.
AtmosphereProvider: A block entity that has the ability to create breathable spaces. It keeps track of the precise places that it provides air for and can be queried.A
Levelstill keeps track of whether it has a breathable atmosphere, but now it won't bother checking for positional breathability if it does. This removes the ability to make "vacuums" (non-breathable spaces) on normally breathable planets, but that functionality wasn't in use anyways.Should a
Levelnot be breathable, then it queries theChunkat the given x/z-coordinate, which then delegates to theChunkSectionat the given y-coordinate. The section returns a list of positions forAtmosphereProviders which will then be queried (for the block entity). If any provider says that the position is breathable, it's considered breathable."Wall" and other solid blocks that are in or surround sealed spaces now are marked breathable.
Unsolved bits
AtmosphereProviderlookup actually need to have per-block-resolution? No, dropped.AtmosphereProviderto a sectionThe future
There's a few neat things that could come out of this in the future, but aren't implemented here for simplicity:
I've also been thinking about sealed spaces unattached to provider blocks (e.g. if a oxygen sealer is broken the air shouldn't just vanish), but that could get more complicated w.r.t block changes, composition/splitting.