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

Pipe/cable overhaul; Fluidfilter fix, GC Energy Compat & Misc fixes #1495

Merged
merged 7 commits into from
Apr 27, 2019
Merged

Pipe/cable overhaul; Fluidfilter fix, GC Energy Compat & Misc fixes #1495

merged 7 commits into from
Apr 27, 2019

Conversation

mitchej123
Copy link
Contributor

Changes from downstream (GTNH fork)

Pipes/Cables - Player visible:

  • GT6 style cables changed default to on
  • Shift Clicking while placing a GT machine will now try connecting to the cable/pipe it is placed on
  • You can open a connection to the air for pipes & wires, allowing the next thing you place down to auto connect (ie: a JABBA barrel)
  • Distribute Fluids - Modeled after several of the upstream PRs
  • Fluid regulators on pipes should stop spazzing out now
  • Fluid filter covers - Now work with filtering output
  • Spray paint on wires/pipes/machines triggers a disconnection check
  • Placing a block next to a wire/pipe triggers a disconnect check
  • Shutters & machine control covers will no longer visibility disconnect the pipe/cable - but will continue to function at blocking the input/output

Backend changes:

  • Unified connect() method for pipes/wires - each subclass has it's own canConnect(), letsIn(), and * letsOut() methods that map to the specifics for that implementation
  • Rewrote the canConnect() logic to be more readable - should be roughly equivalent to the old logic

GC Energy Compat

  • GT cables now properly fill GalacticCraft machines with Energy

IC2/AE2 Energy Compat

  • Updated IC2 & AE2 energy compatibility
  • Added an option ic2EnergySourceCompat (default is on) to allow GT cables to pull energy directly from IC2 energy sources (nuclear reactors, MSFUs, etc) without the need for a transformer
  • Filling IC2/AE2 energy buffers will now send multiple amps if needed

Misc

  • Use a set instead of an arraylist for transfer electricity; deprecated backwards compatible method left in

Copy link
Collaborator

@draknyte1 draknyte1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested & works as expected.

@mitchej123
Copy link
Contributor Author

Added fix to work with subclassed gt cables (GT++, for example), and better GC Compat (shouldn't require GC loaded to boot now)

@SapientMC
Copy link

@mitchej123 Will it also allow to transfer several amps through AE 2p2 tunnel?

Spartak1997 added a commit to Spartak1997/GregTech5.09UnofficialBuild that referenced this pull request Sep 17, 2018
Changes from downstream (GTNH fork)

See more in Blood-Asp/GT5-Unofficial#1495
@mitchej123
Copy link
Contributor Author

@SapientMC I believe AE GT-EU p2p tunnels already handled multiple amps, it was the AE/ic2 recipients that didn't.

If it doesn't work let me know with a setup that breaks.

@SapientMC
Copy link

SapientMC commented Sep 17, 2018

@mitchej123 Could you please build and upload somewhere jar-file with this PR for test?

…s to remove the default methods, but then it's less backwards compatiable)
@mitchej123
Copy link
Contributor Author

@SapientMC
Copy link

@mitchej123 Crash while loading MC
https://pastebin.com/bVsxQJky

@leagris
Copy link
Contributor

leagris commented Sep 18, 2018

@mitchej123 Crash while loading MC
https://pastebin.com/bVsxQJky

Caused by: java.lang.UnsupportedClassVersionError: gregtech/GT_Mod : Unsupported major.minor version 52.0

ByteCode has been compiled with Java 1.8 (version 52.0) because this PR uses Java 1.8 features

But you are running Java 1.7
Java Version: 1.7.0_79, Oracle Corporation

Java 1.7 can not load gregtech/GT_Mod compiled for Java 1.8
...

Upgrade your Java to 1.8 and try again

@OvermindDL1
Copy link

Upgrade your Java to 1.8 and try again

That's not always possible on all systems. Many Mac's for example are capped at 1.7, that is why Forge mandated nothing above 1.7. Consequently you should not compile any mod at a source/target level above 1.7.

@mitchej123
Copy link
Contributor Author

Removed the defaults & compiled against 1.7 - https://app.box.com/s/ang4k1099bvupv2mn8ctvqcct54gjtoz

@Pilad
Copy link

Pilad commented Sep 19, 2018

@mitchej123
Without the GC mode we get crash. Can not find an api mod GC .You need to add a check for GC.
https://pastebin.com/b3Z6gJf7

@OvermindDL1
Copy link

@Pilad By GC do you mean GalactiCraft? I have it on the GT6 maven server if you need to compile against it. Once you have the GT6 maven in your repository list just use (I only have the one version, but it is the latest/last version):

    compileOnly "micdoodle8.mods:MicdoodleCore:1.7-3.0.12.504:Dev"
    compileOnly "micdoodle8.mods:GalacticraftCore:1.7-3.0.12.504:Dev"
    compileOnly "micdoodle8.mods:Galacticraft-Planets:1.7-3.0.12.504:Dev"

No I don't know why it uses a Dev instead of a dev classifier, but that's how they do it... They do not have just an api jar but the dev jar includes the api (as well as everything else if you need to reach in to internals).

@OvermindDL1
Copy link

If you need the GT6 maven repo setup (it's on the GT6 site, but for ease of use):

    maven {
        name = "gt"
        url = "https://gregtech.overminddl1.com/"
    }

I have a SUBSTANTIAL amount of the latest versions of a lot of 1.7.10 mods for pure compiling purposes that GT6 and some other things touch and/or need, so if you need another mod feel free to ask if I have it as there is no list otherwise. Some of the mods are so broken that I had to neuter them so they are not useful to run with (but consequently they do nothing if you runClient at dev time either) but all their API's and such are complete.

@draknyte1
Copy link
Collaborator

As is, our buildscript is broken, since curse changed their stuff a while back. Manual lib placement for compile has been the standby solution. This is good to know that most exist already on your maven repo.

@OvermindDL1
Copy link

As is, our buildscript is broken, since curse changed their stuff a while back.

I would say something about that's what you get for relying on curse, but we all knew they'd do stuff like that. ;-)

Manual lib placement for compile has been the standby solution.

That sounds horrifying... o.O

This is good to know that most exist already on your maven repo.

Really though, I probably have everything you need and a LOT more. If you can give me a list of mods you depend on then I can supply a list of dependencies for them from the GT6 repo. :-)

@mitchej123
Copy link
Contributor Author

Chatted with him offline, he was compiling it himself and only pulled the first three commits. The crash he saw was fixed in the fourth commit already.

The gt6 maven repo is helpful thanks!

@draknyte1
Copy link
Collaborator

https://github.com/Blood-Asp/GT5-Unofficial/blob/unstable/build.gradle

My gradle skills rely on butchering other peoples build scripts.

@Blood-Asp
Copy link
Owner

As i notice that here is again some activity i want to give some lifesigns too.

Currently really busy with work and rl, but i still want to do merges, nessary bugfixes and new builds. If there are important and really tested pull requests i can be reached in discord. And only merge - build - upload can be done in 15min anytime.

@SapientMC
Copy link

@mitchej123

If it doesn't work let me know with a setup that breaks.

It does not
image
P2P Test.zip

@Pilad
Copy link

Pilad commented Sep 30, 2018

@mitchej123
If the option is on(B:GT6StyledWiresConnection=false), the wire is permanently reconnected. If separated by a foil or plate. But if the option is enabled B: GT6StyledWiresConnection = true, then already connected wires are not disconnected by a plate or foil. Maybe it's visually so.

@draknyte1
Copy link
Collaborator

So where are we at with this @mitchej123?
Has more been done that we can rebase this around?

@mitchej123
Copy link
Contributor Author

There is nothing left to do on this except rebate and merge, unless someone has a bug or regression. Everything is in a better state than without this PR afaik. (Ie: bugs fixed and no new known bugs introduced that I am aware of). It's been in use on the gtnh fork for months now

@Dimach
Copy link
Contributor

Dimach commented Feb 8, 2019

@mitchej123 how about bug that reported by Pilad (wires separation by a foil with GT6StyledWiresConnection = false)?
Also some people reported problem with ic2EnergySourceCompat option, its generate lags on large bases (but i don't tested it by my self).

@mitchej123
Copy link
Contributor Author

Foils do not disconnect gt6 style wires, they just stop them from transmitting. Foils worked just fine in my testing with the option enabled and disabled. If someone sends me a non working setup I'll investigate.

I haven't seen any reported problems with the ic2 energy compat, can you point them out to me? (Can always disable it by default)

P2p are in no worse of a position than previously afaik. If there is a regression I'll look into it. If existing issues then I'll pass for now.

@Dimach
Copy link
Contributor

Dimach commented Feb 8, 2019

https://drive.google.com/open?id=1hUVr_W_RCVfmDKuHE2TENK_JD2EWePHm
This reconnection visual, wires not transmit energy (as expected). If you don't find problem i can help with it.
What version you use in testing - vanilla GT or some fork?

@mitchej123
Copy link
Contributor Author

Thanks, the visual is very helpful. I'm out of town right now, but will look into it when back. This is with gt6wires = false; correct?

I normally run the GTNH fork, but tested this on a small pack with GT5u, gt++, Galacticraft, and a few other mods.

@Dimach
Copy link
Contributor

Dimach commented Feb 8, 2019

Yes, gt6wires = false.

@Blood-Asp Blood-Asp merged commit 73ee102 into Blood-Asp:unstable Apr 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants