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

Conduits appear to ignore canInsertItem and isItemValidForSlot #2068

Closed
1Matthias opened this issue Mar 9, 2015 · 23 comments
Closed

Conduits appear to ignore canInsertItem and isItemValidForSlot #2068

1Matthias opened this issue Mar 9, 2015 · 23 comments

Comments

@1Matthias
Copy link

I was using EnderIO conduits for an RotaryCraft extractor setup, and noticed that ore flakes went into the second slot, which should not happen normally. I had the conduits set to In/Out, and ore and flakes in the same chest. I am on MC 1.7.10 and EnderIO 1.7.10-2.2.8.344.
http://imgur.com/7knK5qR,oK1zu23

@HenryLoenwind
Copy link
Member

I've rewritten the inserting code for 2.3, and while the old code needed rewriting, it called canInsertItem() (for ISidedInventory) and isItemValidForSlot() (for IInventory) just fine.

Could it be that RotaryCraft only implements isItemValidForSlot() but is in fact an ISidedInventory?

@ReikaKalseki
Copy link

Could it be that RotaryCraft only implements isItemValidForSlot() but is in fact an ISidedInventory?

No.

Furthermore, my canInsertItem() method bounces to isItemValidForSlot, because I place no sided restrictions, only on slots:
https://github.com/ReikaKalseki/RotaryCraft/blob/master/Base/TileEntity/InventoriedPowerReceiver.java#L79

@yoshiman22
Copy link

Why the hell would you implement ISidedInventory if you don't put side restrictions on it? Rather than create fake accounts and try to get others to take the blame for your mistakes, how about you actually learn to code properly? :)

@tterrag1098
Copy link
Member

You realize that's an abstract base class that can have its methods
overriden for extra functionality, right? It's perfectly valid code. Take
your hate somewhere else unless you have anything of value to add.
On Mar 10, 2015 2:19 PM, "yoshiman22" notifications@github.com wrote:

Why the hell would you implement ISidedInventory if you don't put side
restrictions on it? Rather than create fake accounts and try to get others
to take the blame for your mistakes, how about you actually learn to code
properly? :)


Reply to this email directly or view it on GitHub
#2068 (comment).

@1Matthias
Copy link
Author

Please don't even start, yoshiman22. We are trying to fix a fix a potential bug, not start a flamewar.

@1Matthias 1Matthias reopened this Mar 10, 2015
@1Matthias
Copy link
Author

And I misclicked. Oops.

@HenryLoenwind
Copy link
Member

I'm a bit puzzled by this. However, as I rewrote the code for 2.3 I'm personally not inclined to spend much effort on it for 2.2.8. @1Matthias, would you be willing to test if the same thing happens with a current development version of Ender IO? e.g.:

http://ci.tterrag.com/job/EnderIO/lastSuccessfulBuild/artifact/build/libs/EnderIO-1.7.10-2.2.8.355.jar

But please back up your world before trying. There are some changes that make downgrading again problematic.

@yoshiman22
Copy link

If there's a bug here, it's on Reika's end. That's how it works. :)

@HenryLoenwind Don't bother trying to test or fix this, just say that Reika is not supported like the rest of us.

@1Matthias
Copy link
Author

I am unfortunately playing on a server, which means that I have a hard time updating at the moment. I will try however, and don't worry about my worlds. They are all tester, creative worlds.

@theoanden
Copy link

@yoshiman22 they are tring to fix a potential bug, so please if you arent gonna add anything even remotely useful for the testing of this bug, STOP commenting on the issue.

@1Matthias
Copy link
Author

After this point, I think we should stop commenting on, responding to, or mentioning yoshiman22. Feeding trolls is pointless and a waste of valuble time that could be spent fixing bugs, exploring for diamonds, attemping to hold the tides back with a spoon, etc. yoshiman22, I reccomend you just stop while you are ahead.

@ReikaKalseki
Copy link

@1Matthias Has the new EiO version rectified the issue?

Also, @HenryLoenwind I would check isItemValidForSlot and canInsertItem for ISidedInventories, as one may not encapsulate the other. While I just bounce the latter to the former, I can easily imagine tiles using the former for item/slot verification and the latter for side/slot verification.

@HenryLoenwind
Copy link
Member

Calling both would really be easy, I just didn't want to change the existing logic without major insight into such details. @tterrag1098 Any thoughts on this? I'd update #1897 if it's ok with you.

@1Matthias
Copy link
Author

Just tested it with most up to date build of EIO and RoC, same issue. But there was an odd behavior that might help pin down the issue. The conduits ONLY put the flakes in that one slot, and none of the others.
2015-03-11_19 37 40
2015-03-11_19 37 38
The ones in the second slot are from the items moving automaticly to go to the next stage. I can make a video of it if you want.

@MatthiasMann
Copy link
Contributor

Item conduits work fine with all other machines I have tried so far.
And calling both methods would reduce the performance.

@ReikaKalseki
Copy link

The ones in the second slot are from the items moving automaticly to go to the next stage

Yes, that is automatic Extractor behavior.

calling both methods would reduce the performance

Minimally to the point that you could probably not even profile it.

@MatthiasMann
Copy link
Contributor

When you have a larger item sorting system these methods can be called a lot of times per tick.
I don't see the point in changing EnderIO's code when it works with every other inventory / machine.

@ReikaKalseki what does canInsertItem return for your slots?
When ever ISidedInventory is implemented it's methods are used instead of the ones from IInventory as they allow finer control over the slot interaction - and that works fine.

@ReikaKalseki
Copy link

@ReikaKalseki what does canInsertItem return for your slots?

As said before, it bounces to isItemValidForSlot which actually does the validation.
https://github.com/ReikaKalseki/RotaryCraft/blob/master/TileEntities/Processing/TileEntityExtractor.java#L460

Also, I know this to be a result of a change within EiO, as I do not see it in my instance with an older EiO.

@1Matthias
Copy link
Author

Also, just to put this out there, I have tested this with only RoC and EIO, nothing else. I even got rid of NEI for the sake of testing.

@MatthiasMann
Copy link
Contributor

@ReikaKalseki well - if you call isItemValidForSlot - why should there be any change when EIO would call that too? And which versions are working and which not?

@ReikaKalseki
Copy link

well - if you call isItemValidForSlot - why should there be any change when EIO would call that too

As I said before:

While I just bounce the latter to the former, I can easily imagine tiles using the former for item/slot verification and the latter for side/slot verification.

And which versions are working and which not

I am using 2.2.8.349 with no issues. OK, so it is apparently newer than 1M's version.

@1Matthias
Copy link
Author

By the way, it looks like this bug only occurs with conduits in In/Out mode, not insert mode. Just an odd side note.

@ReikaKalseki
Copy link

By the way, it looks like this bug only occurs with conduits in In/Out mode, not insert mode. Just an odd side note.

I only use insert, which might explain why I do not have it.

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

No branches or pull requests

8 participants