Skip to content

Add isstackvalid check to furnace types#1322

Merged
me4502 merged 3 commits intoEngineHub:masterfrom
RasmusKD:master
Feb 18, 2024
Merged

Add isstackvalid check to furnace types#1322
me4502 merged 3 commits intoEngineHub:masterfrom
RasmusKD:master

Conversation

@RasmusKD
Copy link
Copy Markdown
Contributor

Removes unnessesary lag caused by empty furnaces checking the pipe system

Removes unnessesary lag caused by empty furnaces checking the pipe system
add 5 seconds of delay on pipes if they fail to move a stack, returns to normal behaviour when stack pipe is able to transfer items
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.*;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please revert these formatting changes

private void searchNearbyPipes(Block block, Set<Vector> visitedPipes, List<ItemStack> items) {
Deque<Block> searchQueue = new ArrayDeque<>();
searchQueue.addFirst(block);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This one too


if(!EventUtil.passesFilter(event)) return;

//System.out.println("Pipe triggered");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove this debug line


if(!EventUtil.passesFilter(event)) return;

//System.out.println("Pipe triggered 2");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And this one?

import java.util.*;

public class Pipes extends AbstractCraftBookMechanic {
private Map<String, Long> lastFailedAttempt = new HashMap<>();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this guaranteed to be cleared of all added entries? This could easily cause a pretty major runaway memory problem


String pipeKey = getPipeKey(block);
long currentTime = System.currentTimeMillis();
if(lastFailedAttempt.containsKey(pipeKey) && (currentTime - lastFailedAttempt.get(pipeKey) < 5000)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall I don't feel this is something that can actually be added as is, without a configuration option.

If this does actually make sense (which is probably questionable), it'd have to be behind a configuration option as this will break most currently built systems that rely on any form of timing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sorry didnt mean to add that part

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm happy for the original change of checking if the stack is valid before performing other logic, as that's a simple fast return optimisation - but the rest of it needs more thought and couldn't be merged as-is

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i reverted it

@me4502 me4502 merged commit 0d6cc3d into EngineHub:master Feb 18, 2024
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