ReikaKalseki / Reika_Mods_Issues

The issue tracker for all of my mods - RotaryCraft, its addons, ChromatiCraft, and everything else.
46 stars 14 forks source link

Plasma failed to pass through the coil using verson v7,v8 #217

Closed IamAchang closed 9 years ago

IamAchang commented 9 years ago

Using verson v7,v8 qq 20150901142727

Using verson V6 qq 20150901135630

The same structure leads to different situations.My friends have this same problem too.I do not know why.Is that a bug?or some details have been modified?

Sorry for my poor English. =3

zemerick commented 9 years ago

Recheck your toroids. It does work in v7. Waiting for v8 to settle down before I test that one, but I don't know of any changes that should affect this.

Sometimes it does take some fiddling. This is usually caused by 1 ( or more ) toroids failing their check, and passing this on to the others. Reika had to make some tweaks to deal with exploits, and it will probably take a bit of time to get it behaving really well again, as well as time for all of us to learn exactly how to do it reliably.

zemerick commented 9 years ago
Apparently I lost my detailed post somewhere, so here's a summary, because I don't feel like retyping it. Reika, I seem to have found the source of the issue, but not sure about the specific code. There's actually 2 issues. First, if you build the solenoid first, the Toroids don't get properly updated. I had to break a block and replace it. The bigger one, is the "next" ( hasNext in code ) tag isn't being set properly. 1 toroid will have it, but when spinning it to update it loses it instead of adding to others. Restarting the world causes a fresh update that sets things properly.
zemerick commented 9 years ago

Ok, looking through the code, the only time hasNext can ever be set to true is during the first tick. From there on, it can only be set to false, preventing any toroid from updating.

Update entity used to be able to, but the hasNext part was commented out:

if (reCheckTimer.checkCap() && !world.isRemote) { //hasNext = this.checkCompleteness(world, x, y, z); }

checkCompleteness has the only other time hasNext is set, and it only sets it to false:

((TileEntityToroidMagnet)te).hasNext = false;

I would also like to request that the checkCompleteness "i" be increased to 105 to allow the max-reactor to still work:) ( I believe anything bigger than 104 components will have parts outside of the 100 distance, causing plasma to despawn. So, no point in anything bigger. )

ReikaKalseki commented 9 years ago

I must have commented it out for a reason...

zemerick commented 9 years ago

I think you meant to set hasNext to true inside checkCompleteness, but that change didn't make it. ( Guess obviously. It would make sense to set hasNext within that check. The 2 are directly connected.) You could then also make the same exact change in the first tick check. ( Commenting out the hasNext= part )

I also had a minor idea. Putting:

reCheckTimer.update(); if (reCheckTimer.checkCap() && !world.isRemote) { //hasNext = this.checkCompleteness(world, x, y, z); }

( Which is in the updateEntity function ) inside:

if hasSolenoid{ }

No need for every toroid to do these checks when they don't have solenoid set. They can't affect the plasma anyways, and that's all hasNext is for. A minor performance change, but it's simple enough to implement.

Even more minor thought: I would move that section to above the plasma parts since those check canAffect() which checks hasNext. ( So it'd be nice to update hasNext before that each tick. )

ReikaKalseki commented 9 years ago

So..uncommenting the hasNext = so the result of checkCompleteness() sets it?

zemerick commented 9 years ago

Yea, if you can't remember why you did it that way, uncommenting should work fine.

checkCompleteness is just doing a circle around the tokamak to see if it is complete all the way around. ( IE no LINACs ). If it doesn't make a full circle and get back to the same toroid doing the check, it fails.

The problem was just that the check itself doesn't touch hasNext, so only the call can do that. The only call though that allows hasNext to ever be true was the first tick check. ( Hence restarting the server being the only thing that works. ) Uncommenting that part out, will allow hasNext to be set during the once per second reCheck.

If you would like, I could write up the code with some of my simpler above mentioned changes as well. I could even see if I can figure out how to set up a pull request, but I've never done that before:)

ReikaKalseki commented 9 years ago

I have the sinking feeling that was commented out relevant to some of your designs. :P

zemerick commented 9 years ago

Lol, wouldn't surprise me, but nothing springs immediately to mind yet. And you know I'll tell you if it does.

Perhaps it was a part of more changes due to a problem, but just never got finished up. As is though, it only breaks perfectly good tokamaks, and then a restart does the same thing. This is just preventing the hasNext being updated every second.

The existence of hasNext and checkCompleteness is definitely there to break some of my designs. It stops any form of LINAC from working.

Perhaps it was a part of some change to break the compact designs? Those were the ones you were a bit iffy on, as you liked the big ones. I can't think of what could have been intended though. As it is, the compact designs work just as well as the standard design. You just have to restart the world to get either one to work. The max size is currently broken by the 60 component cap, not hasNext. A moderately large reactor would be in the same situation as the compact and standard: It'd work on restart.

The only other major issue I can think of off-hand is being able to remove the solenoid after initial startup. This doesn't change that either, and that's a royal pain ( NTM still requiring the solenoid to have been running at one point. )

hasNext also breaks bringing in plasma from an external injector not in line with the Tokamak. However, the commented code won't change the way that works either.

ReikaKalseki commented 9 years ago

OK, tentatively marked as fixed.