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

Energy conduit power distribution enhancement #7

Closed
wants to merge 6 commits into from

Conversation

CtrlAltDavid01
Copy link

Disclaimer

I made these changes for myself and I'm opening this pull request to give you a chance to consider these changes for the mod.
I'm open to changes if those would make it better suited for the mod.

This is only lightly tested by me. (I'll be testing it in Monifactory)
Requires performance testing with large conduit networks.

Why create a pullrequest here and not enderio?

The problems I'll touch on in this request are not that big of a problem if you don't have a throughput limit.
Without the throughput limit it would require a completely different implementation, so it wouldn't be usable here.

The problem

The default implementation of the energy conduit is good enough when the conduit has infinite throughput,
but the conduits created whit this mod don't and it really shows.
The player has to manage everything about the network as it is dumb and is prone to doing dumb stuff without supervision.

  • The algorithm tends to waste throughput with just self-feeding entities.
  • Doesn't prioritize machines which can only be outputted to and those that can only be extracted from.

Proposed Changes

When using custom conduits I replaced the logic of power distribution completely to make the network smater, the changes are the following:

  • Know when a machine has multuiple connections.
  • Prioritize pulling from extract only and pushing to insert only machines.
  • Push-Pull machines won't feed eachother, they will be pulled from when the network has a power deficit and pushed to when there is excess power.
  • All the machines will receive the same amout of power (when possible), so no machine gets all the power, while an other one is starwed.
  • Power will be pulled from all sources equivalently, there won't be a case where only onew powersource is loaded and all others are ignored (I did this because Thermal Series dynamos waste power when full)

Drawbacks of the algorithm implemented here

A machine with multiple connections will only use one side to extract and insert all the power that all connections can provide.

@rlnt
Copy link
Member

rlnt commented Oct 8, 2024

Thank you for this submission. Improvements to the distribution system are always good.

Before looking at this more, I have a few things to comment.
Please extract logic into separate methods and outside of the mixin class to make this more descriptive (self-documenting code principle). Make use of guard clauses instead of huge else blocks to avoid more indents and to make things more readable. Lastly, the most important part, avoid the use of streams in ticking logic. Some of that code is called 20 times a second and this implementation is an overhead hell.

@CtrlAltDavid01
Copy link
Author

I refactored the code and is more readable now.
Removed as many streams as possible.

I do understand that this requires substantially more resources than the base implmentation,
but I don't see a way to reduce the computation without compromising on some feature of this addition.

If enderio would notify us in some way when the graph changes some of it could be only computed then, but it's not substantial.

@CtrlAltDavid01
Copy link
Author

Well, I thought about it more and researched other solutions to the problem and I realized this is not the best one
and is wasteful resource wise.

I might try to implement a better method later,
but for now I'm closing this as it is not good enough,

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.

2 participants