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

Rework ae2 mana transfer methodology, update forge version, increase interoperability, and fix a crash #75

Open
wants to merge 6 commits into
base: dev/1.20.1
Choose a base branch
from

Conversation

Vruk11
Copy link

@Vruk11 Vruk11 commented Nov 6, 2024

Sorry if this PR is done poorly in any way, if there's anything that needs fixing let me know.

Reworks the methodology for exporting mana to ManaReceivers to increase interoperability with other Botania addons and fix issue #64

Bumps forge version to be equal to the forge version required of Botania and Patchouli (the game wouldn't launch on forge without this for me)

Fixes #73 by preventing mana P2P tunnels from using a negative amount of energy for spark transfers.

I'm not sure but possibly having this replicated for other version branches would be ideal as well? But I don't want to go and make a bunch of PRs for each branch if I can avoid it.

Prevents forge from hanging on startup in test environment at minimum
This increases intercompatibility and fixes ramidzkh#64
Forgot the fabric version
Forgot to run spotless
@ramidzkh
Copy link
Owner

ramidzkh commented Nov 7, 2024

Thanks for looking into these issues!

I'm not sure but possibly having this replicated for other version branches would be ideal as well? But I don't want to go and make a bunch of PRs for each branch if I can avoid it.

If this gets in (probably, I'll review in a few days though, busy), I can backport it myself

@Vruk11
Copy link
Author

Vruk11 commented Nov 7, 2024

Thanks for looking into these issues!

I'm happy you took the time to even respond to my PR.
I only looked into it initially because my friends and I had issues in our ATM9 server and I wanted to fix them.

If this gets in (probably, I'll review in a few days though, busy), I can backport it myself

I recommend looking at the export bus strategy first as I commented the process most heavily there and replicated the same/similar approach in other parts of the code

The change of methodology that I went with isn't necessarily ideal in every scenario and possible interaction, but I feel it is better than having a specific implementation for each block to find it's capacity. Also I think it is closer to how Botania intends ManaReceivers to be utilized based on what I saw in it's code.

But I would understand if you think this that change is too much, so let me know what you think whenever you get around to looking at it.

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

Successfully merging this pull request may close these issues.

Mana P2P crash
2 participants