[Mod Interaction] How to detect an O2 sphere without a class dependency or heavy reflection?

Discussion in 'Support' started by Reika, Apr 5, 2015.

  1. Reika

    Reika Member

    Joined:
    Apr 5, 2015
    Messages:
    9
    Likes Received:
    0
    My airbreathing engines check the environment to ensure that they have air, and if the world happens to be an IGalacticCraftWorldProvider, they check for IAtmosphericGas oxygen. However, this apparently returns false even inside an "oxygen dome", which is unintended functionality on my end; I am trying to prevent things like jet engines from working in a vacuum, not just "on the moon".

    I have a few people - one particularly incessantly - asking for some sort of compatibility, but I know of no way to do this without either heavy reflection or a direct class reference, the former of which I would really strongly prefer to avoid and the latter of which is entirely nonviable.

    Is there a way through the GC API to detect such a thing (and, if such a mechanic exists in GC, deplete a portion of that oxygen)?
     
    #1 Reika, Apr 5, 2015
    Last edited: Apr 5, 2015
  2. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
    Hey Reika.

    Would the methods in OxygenUtil cover what you need?
    https://github.com/micdoodle8/Galac...8/mods/galacticraft/core/util/OxygenUtil.java

    Especially, if checking a tile that needs oxygen I would in general use this:
    Code:
    if (OxygenUtil.noAtmosphericCombustion(worldObj.provider) && OxygenUtil.isAABBInBreathableAirBlock(worldObj, boundingBox))
                  //Code for no oxygen present
    
    The isAABBInBreathableAirBlock covers everything, either the Oxygen Bubble Distributor or an Oxygen Sealer (including when oxygen from the sealer can reach the block through other permeable blocks).

    That's not in the API though - I guess nobody ever asked for it before...

    If you want, we can easily put access for this into the API for our next build, but then your mod will not be compatible with any version of Galacticraft prior to that update.

    Alternatively you could access those two methods through reflection? We will not change them if we know you are using them like that. I'm happy to write the reflection code if you don't fancy doing it, would only be about 20 lines.

    We do not have a mechanic for consuming oxygen - maybe we will in future (in which case I guess the existing oxygen present test methods in OxygenUtil can also consume it - as any call to those test methods would have to be an update tick for a torch, machine or mob using oxygen).
     
  3. Reika

    Reika Member

    Joined:
    Apr 5, 2015
    Messages:
    9
    Likes Received:
    0
    Yes, putting it or some hook to it in the API would be greatly appreciated.

    I can do but would rather avoid reflection, because reflective handlers tend to break constantly (just look at my MystCraft and ThaumCraft handlers) as the other mod refactors and updates.
    Though if there is no way to check the GC version, it may be the only choice...
     
  4. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
  5. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
    I would test whether that class is present, or not. If not present then you know it's an old GC version (or some other mod ... ahem Reika! ... force-loaded an out-dated copy of the GC API before Galacticraft loaded).

    You can do a version check for GC 3.0.12 which will include this API change, but eventually we will move on to Galacticraft 4 or maybe 3.1 and I don't know how the future version numbering will look! So I think the class present check is safer.
     
  6. Reika

    Reika Member

    Joined:
    Apr 5, 2015
    Messages:
    9
    Likes Received:
    0
    Thank you! :D

    One question though: How am I to include this class in the dev environment, seeing as the OxygenUtil class it makes direct reference to does not exist (and if added, would require more classes yet)? I do not want to add the entirety of the GC source, as I need my dev environment able to launch quickly.

    I know I could just make a "dummy" class with empty methods, but I think that will break testing, and it is also something of an ugly solution.
     
  7. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
    Recommended approach is to include the Galacticraft API sources in your dev environment and also include the three .jars from the deobfuscated dev build of Galacticraft as a library.
    http://ci.micdoodle8.com/job/Galacticraft3-dev/

    I made a new Galacticraft dev build 132 just now, so if you grab that it should now have this new OxygenHooks class in the API.
    Likewise Galacticraft release builds from 314 forwards will have this in.
     
  8. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
    If you want to exclude the Galacticraft .jars from your dev environment again after testing this, then I guess the best approach is in your copy of the API sources just comment out the return lines in OxygenHooks which call OxygenUtil, and have them return dummy values like null or true. As long as you are not distributing the GC API with your mod, that shouldn't cause any problems I guess.
     
  9. Reika

    Reika Member

    Joined:
    Apr 5, 2015
    Messages:
    9
    Likes Received:
    0
    I am willing to add source jars to the build path, but is there a way I can do it that does not forcibly install the mod to the dev environment? My dev env modlist is growing and its launch time is rising.
     
  10. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
    Maybe it's possible for your IDE or project properties to store libraries in a folder which the Minecraft launcher can't see? I'm not sure, never tried it.

    Separately from that: https://github.com/micdoodle8/Galacticraft/issues/1571
    Seems to be connected with ChromaASMHandler in your v6d update, but I'm not an expert on how your mod works. If there's anything I can do to help, please let me know. MicdoodleCore source is here: https://github.com/micdoodle8/Micdo...oodle8/mods/miccore/MicdoodleTransformer.java
    See also: https://github.com/micdoodle8/Micdo...le8/mods/miccore/MicdoodlePlugin.java#L84-116
     
  11. Reika

    Reika Member

    Joined:
    Apr 5, 2015
    Messages:
    9
    Likes Received:
    0
    I will try that, but the thing is, I spoke to Abrar, and it seems including something in the build path as a mod is intentional. I find it stupid, but if that is the way FML wants to work, there is little that can be done.


    In the V6d update, I added some ASM to ChromatiCraft, to the populate() method of ChunkProviderServer. I did this to intercept the call to CPW's "generateWorld" event to run the IWorldGenerators, because I do not want to run them in my custom dimension. What I do specifically is find the INVOKESTATIC to generateWorld and change it to point to a custom function of mine (changed the owner and name parameters; an attempted fix also changed the signature, which is normally unnecessary), which then passes it off to the appropriate code (in all dimensions but mine, it just calls generateWorld). Looking at the errors, it appears that the difference is a method signature. For me (and Forge 1291), populate() is "(IILWorld;LIChunkProvider;LIChunkProvider; )V" - i.e. returns void. In many of these errors, it returns a boolean ("()Z"). Does GC change any of that?
     
  12. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
    On the ASM issue, Galacticraft had exactly the same worldgen problem - seems like we both independently came to a similar solution. See:
    https://github.com/micdoodle8/Micdo...ds/miccore/MicdoodleTransformer.java#L489-509
    https://github.com/micdoodle8/Micdo.../mods/miccore/MicdoodleTransformer.java#L1100

    The effect is to insert into ChunkProviderServer.populate() some bytecode so it now reads like this:

    Code:
                if (this.currentChunkProvider != null)
                {
                    this.currentChunkProvider.populate(p_73153_1_, p_73153_2_, p_73153_3_);
                    if (!micdoodle8.mods.galacticraft.core.util.otherModPreventGenerate(p_73153_2_, p_73153_3_, worldObj, currentChunkProvider, p_73153_1_))
                             GameRegistry.generateWorld(p_73153_2_, p_73153_3_, worldObj, currentChunkProvider, p_73153_1_);
                    chunk.setChunkModified();
                }
    
    So the bytecode is loading up the 5 parameters passed, an INVOKESTATIC, and a IFNE jump to the following line.

    I guess this bytecode injection means your mod is now not finding the exact bytecode in ChunkProviderServer.populate() which it is expecting? You can maybe able to work around that - we have overcome most compatibility issues by having our bytecode injection flexible, so it searches for the correct bytecode to replace instead of assuming an exact position etc.

    Alternatively could you skip that bytecode transform in your mod if Galacticraft is installed, and instead we can add a hook to your mod in otherModPreventGenerate() - just give me the code you want and I'll happily put it in there - for a start I guess it would be a question of not simply returning false at https://github.com/micdoodle8/Galac...s/galacticraft/core/util/WorldUtil.java#L1102.

    ===================================

    On the IDE issue, if you will not actually be distributing the Galacticraft API, then once you have checked it's all working OK, you could simply remove the Galacticraft jars from your library and edit the Galacticraft API sources (your local copy in your IDE) to eliminate compile errors. Anyhow, that's the approach we use - I've had to edit some of the APIs in Galacticraft dependencies (https://github.com/micdoodle8/Galacticraft/tree/master/dependencies/main/java) to make things compile correctly, but we do not distribute any of the dependencies with the mod - they are referenced only if a mod loaded check shows the mod is loaded.
     
  13. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
    hmm, actually I'm not sure whose ASM transformer runs first here - maybe ours now has a IFNE jump to the wrong spot, if your mod is loaded?

    EDIT: no ours should still IFNE jump to the correct spot, because it examines the existing code for a static invoke on a GameRegistry method, when setting the label to jump to. I'm not too sure exactly what's going wrong without looking at your bytecode transform in detail - can you point me to the sources?
     
  14. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
  15. Reika

    Reika Member

    Joined:
    Apr 5, 2015
    Messages:
    9
    Likes Received:
    0
    Here you go - I literally redirect the method:
    https://github.com/ReikaKalseki/ChromatiCraft/blob/master/Auxiliary/ChromaASMHandler.java#L180

    We should probably get this worked into Forge if we can (a new method for the ChunkProvider seems best), but for now, yes, we need a workaround.
     
  16. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
    https://github.com/ReikaKalseki/ChromatiCraft/pull/3
    I hope this will fix it. I think if Galacticraft was installed, your code was redirecting the INVOKESTATIC on the Galacticraft boolean method instead of on GameRegistry.generateWorld().

    Regarding Forge, a change would be great even though it will only help 1.8 versions now, I guess it would have to be an event fired at the start of GameRegistry.generateWorld() which then skips that generation if cancelled. I meant to link you to this before, showing your our hook: https://github.com/micdoodle8/Galac...s/galacticraft/core/util/WorldUtil.java#L1100 so we actually "whitelist" the worldgens from a few mods there.

    Main reason for doing it that way - instead of your way of redirecting the call, which is how I did it first - is I was fed up of seeing Crash Reports from players saying "Galacticraft has crashed" or "Galacticraft is not compatible with X mod" just because our code was in the stacktrace (equivalent of your ChromaAux.interceptChunkPopulation()) when the other mod's worldgen crashed. My guess is you'll see what I mean coming from your players also in the next few months...
     
  17. Reika

    Reika Member

    Joined:
    Apr 5, 2015
    Messages:
    9
    Likes Received:
    0
    I do hope that does not become too common, but I can definitely see it happening - it already has for things like OC networking crashes or my IWorldGenerators (and it is not helped by a few people who say things like "if Reika is mentioned in the log, it must be his fault"). :confused:

    At any rate, I included a modified version of your fix for inclusion later today; hopefully this works.
     
  18. radfast

    radfast Administrator
    Staff Member

    Joined:
    Apr 27, 2014
    Messages:
    1,144
    Likes Received:
    352
    Great, thank you. I'm going to suggest to the two people who reported crashes on the GC issues list that they try your version e.
     
  19. Reika

    Reika Member

    Joined:
    Apr 5, 2015
    Messages:
    9
    Likes Received:
    0
    The first reports are in, and it appears that v6e fixes the issue. :D
     
  20. Reika

    Reika Member

    Joined:
    Apr 5, 2015
    Messages:
    9
    Likes Received:
    0
    As for the O2 API stuff, I have talked with Abrar and there appears to be no way to reference a jar as a library without FML also loading it as a mod (this is intentional FML behavior), which makes the solution rather unfeasible.

    For now, I suppose I will go with creating a dummy OxygenUtil for the API to reference.
     

Share This Page