-
Notifications
You must be signed in to change notification settings - Fork 17
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
Fixes + tunnel colors + transparent latch outputs #179
base: master
Are you sure you want to change the base?
Changes from 4 commits
410c5e8
9450ef8
a2b63d6
c2354b3
c83b6d1
3daee2d
13eeb15
1ce2ee9
869ac47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package moe.nightfall.vic.integratedcircuits.cp.legacy; | ||
|
||
import java.util.ArrayList; | ||
import java.util.BitSet; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
|
||
import moe.nightfall.vic.integratedcircuits.cp.CircuitData; | ||
import moe.nightfall.vic.integratedcircuits.cp.legacy.LegacyLoader; | ||
import moe.nightfall.vic.integratedcircuits.misc.MiscUtils; | ||
import moe.nightfall.vic.integratedcircuits.misc.Vec2; | ||
import net.minecraft.nbt.NBTTagCompound; | ||
import net.minecraftforge.common.util.ForgeDirection; | ||
|
||
public final class LegacyLoader_1_Tunnels extends LegacyLoader { | ||
@Override | ||
public int getVersion() { | ||
return 1; | ||
} | ||
|
||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only a minor thing, but I hate using the initialization blocks (looks a lot cleaner in scala), so rather make it a normal constructor. |
||
addTransformer(new PartTunnelTransformer(), 27); | ||
} | ||
|
||
private static class PartTunnelTransformer extends PartTransformer { | ||
// The old PartTunnel property map | ||
// PartCPGate: | ||
protected final int oldInput = old.allocate(4); | ||
// PartTunnel: | ||
protected final int oldPosX = old.allocate(8); | ||
protected final int oldPosY = old.allocate(8); | ||
protected final int oldIn = old.allocate(); | ||
|
||
// Tunnels are now derived from wires | ||
// PartCPGate: | ||
protected final int newInput = transformed.allocate(4); | ||
// PartWire: | ||
protected final int newColor = transformed.allocate(2); | ||
// PartTunnel: | ||
protected final int newPosX = transformed.allocate(8); | ||
protected final int newPosY = transformed.allocate(8); | ||
protected final int newIn = transformed.allocate(); | ||
|
||
@Override | ||
public void transformImpl() { | ||
setInt(newInput, getInt(oldInput)); | ||
// Old tunnels are converted to green tunnels | ||
setInt(newColor, 0); // 0 is green | ||
setInt(newPosX, getInt(oldPosX)); | ||
setInt(newPosY, getInt(oldPosY)); | ||
setBit(newIn, getBit(oldIn)); | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,10 +35,17 @@ public boolean getOutputToSide(Vec2 pos, ICircuit parent, ForgeDirection side) { | |
|
||
@Override | ||
@SideOnly(Side.CLIENT) | ||
// This renders BOTH WIRES AND TUNNELS | ||
public void renderPart(Vec2 pos, ICircuit parent, double x, double y, CircuitPartRenderer.EnumRenderType type) { | ||
int color = this.getColor(pos, parent); | ||
boolean tun = this instanceof PartTunnel; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nah, find a OOP way of doing this, instanceof is an antipattern There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Properly using OOP is hell. I mean designing a proper class hierarchy. I know that this check is actually Oh wait! Call super is antipattern too! So this should actually be a And what about In general, hardcoding is bad, but avoiding hardcoding takes time and decreases runtime performance, while absolutely no hardcoding means infinite code. I do not mean that I reject your suggestion to "find an OOP way of doing this", just that I see no practical reason to not simply use The code base has a ton of cleanup potential, and would likely benefit from more comments and documentation. But I guess no one has time to refactor it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There have been a lot of requests to enable wire-like dragging for other parts as well so I think this should be something that's not bound to PartWire, specifically. Otherwise I pretty much agree with you, and yes, the parts should handle rendering inside the GUI as well, instead of that one giant method I use to do that, mostly replicating already existing render code. But getting started on that would take a lot of time, and I still have a port to 1.8.9 somewhere in the back of my head so I don't want to put too much refactoring effort into this. Its a lot simpler to point these things out in PRs, doesn't mean I'm any better at it myself ;) |
||
Tessellator tes = Tessellator.instance; | ||
|
||
if (tun) { | ||
RenderUtils.applyColorIRGBA(tes, Config.colorGreen); | ||
CircuitPartRenderer.addQuad(x, y, 16, 4*16, 16, 16); | ||
} | ||
|
||
if (type == CircuitPartRenderer.EnumRenderType.GUI) { | ||
switch (color) { | ||
case 1: | ||
|
@@ -66,9 +73,9 @@ public void renderPart(Vec2 pos, ICircuit parent, double x, double y, CircuitPar | |
int ty = type == CircuitPartRenderer.EnumRenderType.WORLD_16x ? 3 * 16 : 0; | ||
|
||
int con = CircuitPartRenderer.checkConnections(pos, parent, this); | ||
if ((con & 12) == 12 && (con & ~12) == 0) | ||
if ((con & 12) == 12 && (con & ~12) == 0 && !tun) | ||
CircuitPartRenderer.addQuad(x, y, 6 * 16, ty, 16, 16); | ||
else if ((con & 3) == 3 && (con & ~3) == 0) | ||
else if ((con & 3) == 3 && (con & ~3) == 0 && !tun) | ||
CircuitPartRenderer.addQuad(x, y, 5 * 16, ty, 16, 16); | ||
else { | ||
if ((con & 8) > 0) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the legacy loader doesn't work at all (because it doesn't do the steps in sequence, there's a big TODO on it), so if you decide to change the version it should probably be fixed first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These steps are NBT, transform and postTransform. For what I did this is fine.
Anyway, the proper solution is to do conversion using separate "frozen" codebase that will never have to be changed. That is, no
CircuitData
or any other such classes. It should receive NBT and produce new NBT.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess... Still would like to see that get fixed though