Posted June 21, 20178 yr My lamp has a tile entity that updates its lighting depending on how much power it holds. The lighting doesn't update properly, though. When I place the lamp it is at full brightness, if it's power level reduces it goes a tad bit darker, but not mutch. It never completely goes black. When I break the block the lighting doesn't update and lingers until I restart the world. What am I doing wrong? BlockLamp public class BlockLamp extends Block implements ITileEntityProvider { int capacity; int loss; float lifetime; float lowEnergyMultiplier; float mediumEnergyMultiplier; float highEnergyMultiplier; public BlockLamp(Material material, String name, float lightLevel, float lifetime, int capacity, int loss) { this(material, name, lightLevel, lifetime, capacity, loss, 1, 2, 3); } public BlockLamp(Material material, String name, float lightLevel , float lifetime, int capacity, int loss, float lowEnergyMultiplier, float mediumEnergyMultiplier, float highEnergyMultiplier) { super(material); this.setUnlocalizedName(name); this.setRegistryName(name); this.setCreativeTab(BrightenUp.tabBrightenUp); this.setLightLevel(lightLevel); this.loss = loss; this.capacity = capacity; this.lifetime = lifetime; this.lowEnergyMultiplier = lowEnergyMultiplier; this.mediumEnergyMultiplier = mediumEnergyMultiplier; this.highEnergyMultiplier = highEnergyMultiplier; } @Override public TileEntity createNewTileEntity(World worldIn, int meta) { return new TileEntityLamp(capacity, loss, lifetime, lowEnergyMultiplier, mediumEnergyMultiplier, highEnergyMultiplier); } @Override public void breakBlock(World worldIn, BlockPos pos, IBlockState state) { if(worldIn.getTileEntity(pos) instanceof TileEntityLamp) { worldIn.removeTileEntity(pos); } } public float getBaseLightValue() { return this.lightValue; } } TileEntityLamp public class TileEntityLamp extends TileEntity implements ITickable, IEnergyReceiver { EnergyStorage storage; int loss; float lifetime; float lowEnergyMultiplier; float mediumEnergyMultiplier; float highEnergyMultiplier; public TileEntityLamp(int capacity, int loss, float lifetime, float lowEnergyMultiplier, float mediumEnergyMultiplier, float highEnergyMultiplier) { storage = new EnergyStorage(capacity); this.loss = loss; this.lifetime = lifetime; this.lowEnergyMultiplier = lowEnergyMultiplier; this.mediumEnergyMultiplier = mediumEnergyMultiplier; this.highEnergyMultiplier = highEnergyMultiplier; } @Override public void update() { if (this.worldObj.getBlockState(this.pos).getBlock() instanceof BlockLamp) { BlockLamp lamp = (BlockLamp) this.worldObj.getBlockState(pos).getBlock(); EnergyLevel previousLevel = this.getEnergyLevel(); System.out.println(this.getEnergyPercentage()); storage.modifyEnergyStored(-loss); if (this.getEnergyLevel() != previousLevel) { switch (this.getEnergyLevel()) { case EMPTY: lamp.setLightLevel(0); break; case LOW: this.lifetime -= 1 * lowEnergyMultiplier; lamp.setLightLevel(lamp.getBaseLightValue()); break; case MEDIUM: this.lifetime -= 1 * mediumEnergyMultiplier; lamp.setLightLevel(lamp.getBaseLightValue() * 3); break; case HIGH: this.lifetime -= 1 * highEnergyMultiplier; lamp.setLightLevel(lamp.getBaseLightValue() * 5); break; } this.worldObj.notifyBlockUpdate(pos, this.worldObj.getBlockState(pos), this.worldObj.getBlockState(pos), 3); } this.markDirty(); } this.worldObj.checkLightFor(EnumSkyBlock.BLOCK, pos); } @Override public NBTTagCompound writeToNBT(NBTTagCompound compound) { super.writeToNBT(compound); storage.writeToNBT(compound); compound.setFloat("lifetime", lifetime); return compound; } @Override public void readFromNBT(NBTTagCompound compound) { super.readFromNBT(compound); storage.readFromNBT(compound); lifetime = compound.getFloat("lifetime"); } @Override public int receiveEnergy(EnumFacing from, int maxReceive, boolean simulate) { return storage.receiveEnergy(maxReceive, simulate); } @Override public int getEnergyStored(EnumFacing from) { return storage.getEnergyStored(); } @Override public int getMaxEnergyStored(EnumFacing from) { return storage.getMaxEnergyStored(); } @Override public boolean canConnectEnergy(EnumFacing from) { return from == EnumFacing.DOWN || from == EnumFacing.UP; } private float getEnergyPercentage() { return (storage.getEnergyStored() * 100) / storage.getMaxEnergyStored(); } public EnergyLevel getEnergyLevel() { if(this.getEnergyPercentage() <= 40) { return EnergyLevel.LOW; } else if(this.getEnergyPercentage() > 40 && this.getEnergyPercentage() < 60) { return EnergyLevel.MEDIUM; } else if(this.getEnergyPercentage() <= 0) { return EnergyLevel.EMPTY; } else { return EnergyLevel.HIGH; } } public enum EnergyLevel { EMPTY, LOW, MEDIUM, HIGH } } I have also tried having the block handle the update, but that had a similar result. Any ideas?
June 21, 20178 yr 20 minutes ago, That_Martin_Guy said: ITileEntityProvider Is outdated. Use Block::createTileEntity and Block::hasTileEntity. 20 minutes ago, That_Martin_Guy said: BlockLamp lamp = (BlockLamp) this.worldObj.getBlockState(pos).getBlock(); 20 minutes ago, That_Martin_Guy said: lamp.setLightLevel(0); Blocks are singletons. Tile entities are not. You can't change the value like that as the change will be reflected on all lamps in the world. You can override Block::getLightValue(IBlockState, IBlockAccess, BlockPos) and return the appropriate light level there based on the tile you get at the coordinates. You will also have to call World::checkLight to redraw the light levels around your lamp as you change them as they are not updated automatically for you.
June 21, 20178 yr Author I added @Override public int getLightValue(IBlockState state, IBlockAccess world, BlockPos pos) { if(world.getTileEntity(pos) instanceof TileEntityLamp) { TileEntityLamp tileEntity = (TileEntityLamp) world.getTileEntity(pos); TileEntityLamp.EnergyLevel energyLevel = tileEntity.getEnergyLevel(); switch(energyLevel) { case LOW: return getBaseLightValue(); case MEDIUM: return getBaseLightValue() * 3; case HIGH: return getBaseLightValue() * 5; default: return 0; } } return 0; } to my block and changed the tile entity's update method to @Override public void update() { if (this.worldObj.getBlockState(this.pos).getBlock() instanceof BlockLamp) { EnergyLevel previousLevel = this.getEnergyLevel(); System.out.println(this.getEnergyPercentage()); storage.modifyEnergyStored(-loss); if (this.getEnergyLevel() != previousLevel) { switch (this.getEnergyLevel()) { case LOW: this.lifetime -= 1 * lowEnergyMultiplier; break; case MEDIUM: this.lifetime -= 1 * mediumEnergyMultiplier; break; case HIGH: this.lifetime -= 1 * highEnergyMultiplier; break; } this.worldObj.checkLight(pos); } this.markDirty(); } } . Still has very similar behavior.
June 21, 20178 yr Are you sure that the client has correct energy data for your tile? Try debugging the value of the 13 minutes ago, That_Martin_Guy said: previousLevel local in your getLightValue method. I can't reproduce the issue with a similar code.
June 21, 20178 yr Author Bingo! The client is always at LOW, no matter how much energy the server has. To fix this I encased the contents of update in if(!worldObj.isRemote) and getLightValue in if(FMLCommonHandler.instance().getSide().isServer()) . The problem is that now the block doesn't have lighting at all. Does this mean that the light is checked on the client while the energy level is checked on the server? Big bummer if this is the case.
June 22, 20178 yr Author I don't really understand what to do at this point. I tried adding creating a message for sending the energy level to the client and basing the blocks energy level off of that, but the block updates are wonky and it can be at full brightness when empty. Any ideas? Refer to my github if you want to look at my code.
June 22, 20178 yr Author 40 minutes ago, diesieben07 said: (to convert block coordinates into chunk coordinates use chunkCoord = blockCoord >> 4). Doesn't seem to be possible to convert it like this, unless I'm doing something wrong. I got the method you mentioned this way instead: worldObj.getMinecraftServer().worldServerForDimension(worldObj.provider.getDimension()).getPlayerChunkMap().getEntry(worldObj.getChunkFromBlockCoords(pos).xPosition, worldObj.getChunkFromBlockCoords(pos).zPosition) . The problem is that I can't add .players to the end of this line. Further digging reveals that the only players field in PlayerChunkMapEntry is a private final List<EntityPlayerMP>. How do I access this field? 49 minutes ago, diesieben07 said: Why are you calling markDirty every tick? Woops! Mistake on my part. Don't know when I moved it, but it's only supposed to be called if the energy value actually changed.
June 22, 20178 yr Author 53 minutes ago, diesieben07 said: I forgot to mention this, you will need reflection to access it. Never heard of reflection before, so I looked it up. I tried to find something related to getting a private List with a non-generic type, but I didn't find anything. I'm completely lost here. I tried Class<?> playerEntry = worldObj.getMinecraftServer().worldServerForDimension(worldObj.provider.getDimension()).getPlayerChunkMap().getEntry(worldObj.getChunkFromBlockCoords(pos).xPosition, worldObj.getChunkFromBlockCoords(pos).zPosition).getClass(); try { Field players = playerEntry.getDeclaredField("players"); List<EntityPlayerMP> playerList = players.getDeclaringClass().cast(List<EntityPlayerMP>()); } catch (NoSuchFieldException e) { e.printStackTrace(); } catch (IllegalAccessException e) { e.printStackTrace(); } , but IDEA gives me "Expression expected" under // this here // | | // \/ \/ List<EntityPlayerMP> playerList = players.getDeclaringClass().cast(List<EntityPlayerMP>()); .
June 22, 20178 yr Author Still puzzled trying to figure out how to get List<EntityPlayerMP> from the Field. I have updated my github to the most up to date version, but it crashes with this log.
June 22, 20178 yr This is not how you get a value from a field using reflection. There is a Field::get(Object) method you must call to get a value the field stores. The object parameter is an instance of a class the field belongs to. If the field is static you simply pass null. If not - you need to pass the instance to get the value from. Additionaly you need to call Field::setAccessible(true) before doing anything with a field, as if it is not public you will get your access denied.
June 22, 20178 yr Author I feel like I'm getting close, but I still have a problem. The lighting doesn't update properly (only seems to send network messages when un-charging) and every time the energy level changes the lamp turns on for a split second, then immediatly turns off. Some changes to the code are a new IProperty called PropertyEnum in the block class for the energy level, EnergyLevel is now in a seperate class (thatmartinguy.brightenup.energy.EnergyLevel) and the LampEnergyMessage now requires a string (to be converted into an enum) and a pos instead of an int and a pos. Github
June 24, 20178 yr Author You can definitely tell I was tired when programming this because... Oh god it's horrible. Let's just go through the list shall we? On 2017-06-22 at 10:46 PM, diesieben07 said: But you do not really need this block state property anyways, you can just access the TileEntity data directly in getLightValue. Fully agree. State removed, replaced all the blockstate related methods with this change in getLightValue: public int getLightValue(IBlockState state, IBlockAccess world, BlockPos pos) { if(world.getTileEntity(pos) instanceof TileEntityLamp) { TileEntityLamp lamp = (TileEntityLamp) world.getTileEntity(pos); switch(lamp.getEnergyLevel()) { case LOW: return getBaseLightValue(); case MEDIUM: return getBaseLightValue() * 2; case HIGH: return getBaseLightValue() * 5; } System.out.println(lamp.getEnergyLevel()); } return 0; } and LampEnergyMessage#Handler#onMessage to public IMessage onMessage(LampEnergyMessage message, MessageContext ctx) { FMLCommonHandler.instance().getWorldThread(ctx.getClientHandler()).addScheduledTask(() -> { final World world = Minecraft.getMinecraft().theWorld; assert world != null; if(world.getTileEntity(message.pos) instanceof TileEntityLamp) { TileEntityLamp lamp = (TileEntityLamp) world.getTileEntity(message.pos); lamp.setEnergyLevel(EnergyLevel.valueOf(message.energyLevel.toUpperCase())); world.checkLight(message.pos); System.out.println("Message sent as " + message.energyLevel.toUpperCase()); } }); return null; } . TileEntityLamp#getEnergyLevel and setEnergyLevel are the simplest getters and setters you can get and not really worth posting. On 2017-06-22 at 10:46 PM, diesieben07 said: Your TileEntity moreover again duplicates the "how much energy do I have" information, by having both the EnergyStorage and the EnergyLevel stored. But this is completely broken, since you never write any meaningful value to the energyLevel field. ... Construct the IMessage instance once and then send it to all players. Also, please use an enhanced for loop ("foreach loop") and not this atrocious "i keep track of my own indices" stuff. Thank you for the heads up! Completely missed that I didn't ever change energyLevel. TileEntityLamp#update has been changed to public void update() { if(!worldObj.isRemote) { if (worldObj.getBlockState(this.pos).getBlock() instanceof BlockLamp) { EnergyLevel previousLevel = EnergyLevel.getEnergyLevel(storage.getEnergyStored(), storage.getMaxEnergyStored()); storage.modifyEnergyStored(-loss); if (EnergyLevel.getEnergyLevel(storage.getEnergyStored(), storage.getMaxEnergyStored()) != previousLevel) { switch (EnergyLevel.getEnergyLevel(storage.getEnergyStored(), storage.getMaxEnergyStored())) { case EMPTY: energyLevel = EMPTY; break; case LOW: this.lifetime -= 1 * lowEnergyMultiplier; energyLevel = LOW; break; case MEDIUM: this.lifetime -= 1 * mediumEnergyMultiplier; energyLevel = MEDIUM; break; case HIGH: this.lifetime -= 1 * highEnergyMultiplier; energyLevel = HIGH; break; } try { List<EntityPlayerMP> playerList = (List<EntityPlayerMP>) PLAYER_ENTRY_LIST.get(worldObj.getMinecraftServer().worldServerForDimension(worldObj.provider.getDimension()).getPlayerChunkMap().getEntry(worldObj.getChunkFromBlockCoords(pos).xPosition, worldObj.getChunkFromBlockCoords(pos).zPosition)); LampEnergyMessage message = new LampEnergyMessage(energyLevel.toString(), pos); for(EntityPlayerMP player : playerList) { BrightenUp.network.sendTo(message, player); } } //Hang with me on this one catch (IllegalAccessException e) { e.printStackTrace(); } worldObj.checkLight(pos); worldObj.notifyBlockUpdate(pos, worldObj.getBlockState(pos), worldObj.getBlockState(pos), 3); this.markDirty(); } } } } On 2017-06-22 at 10:46 PM, diesieben07 said: As a side note, why in gods name are all your fields package-visible and not final? Make things final if possible! And why do you duplicate some fields (the energy multipliers) between your TE and your Block? I honestly thought not including the access type in the declaration made it private. TIL. Any fields that could be are now final. I make duplicate values in both my block and TE because I don't know any other way to transfer its properties from the block to the TE (other than setting up setters in a similar way to vanilla minecraft, but IMO that makes the code way too unorganized for my taste). On 2017-06-22 at 10:46 PM, diesieben07 said: What on earth is this construct: (worldObj.getMinecraftServer().worldServerForDimension(worldObj.provider.getDimension())? Don't really know a simpler to do this. The best way to get a MinecraftServer (that I've heard of) is with a World object. I didn't see any other way to call get a WorldServer without MinecraftServer other than worldServerForDimension, although I might be wrong on this. World#provider#getDimension was the only way I found were you could get a dimension ID, but again, I might be wrong. On 2017-06-22 at 10:46 PM, diesieben07 said: Why is your energy level in the packet a String? 'Cause I didn't find any other way to transfer enums through the network, so I figured using toString and valueOf was the second best option. On 2017-06-22 at 10:46 PM, diesieben07 said: Don't just print out exceptions to the console and carry on as if nothing was wrong. I keep it as a simple print since I might re-design my code a lot, and it is just easier to keep it like this in case I need to move or remove it. Will definitely improve it once the lamp light works properly. However, even after all of these changes, I still cannot get the lamp's lighting to work. It doesn't update when a creative energy cell is next to it, only when it would normally update (block next to it destroyed, placed etc). When it is empty it does update, however, and turns off completely. Not when it is HIGH, MEDIUM or LOW, though.
June 24, 20178 yr 5 minutes ago, That_Martin_Guy said: 'Cause I didn't find any other way to transfer enums through the network, so I figured using toString and valueOf was the second best option. Best way is to use ordinal() to turn the enum value into an int, and then values()[ordinal] to turn the int back into the enum value. 11 minutes ago, That_Martin_Guy said: It doesn't update when a creative energy cell is next to it Where is the code which is supposed to make this happen?
June 25, 20178 yr Author 6 hours ago, Jay Avery said: Where is the code which is supposed to make this happen? worldObj.notifyBlockUpdate(pos, worldObj.getBlockState(pos), worldObj.getBlockState(pos), 3); in TileEntityLamp#update? Or is this not how you update blocks? Also, after applying the changes diesieben suggested the block doesn't update its lighting at all, unless by normal means (placing/breaking blocks for example). My github has been updated once again. EDIT: I also feel the LampEnergyMessage is a bit unnecessary at this point, since I can call TileEntityLamp#getEnergyLevel on both sides. I'm not really using the message properly, currently. Not really sure how to set it since the old energyLevel field has been removed. Edited June 25, 20178 yr by That_Martin_Guy
June 25, 20178 yr Author 58 minutes ago, diesieben07 said: Your packet does not do anything. It simply tells the client to re-check for light, but the client still has no idea what the energy value is, so it cannot compute the light properly I'm aware of this. The problem is that I'm not sure what I want it to do. There is no EnergyLevel field, so I can't set that property somehow. I also thought that getEnergyLevel was how I was supposed to set it, since it could be called on both sides, but I guess I'm wrong.
June 25, 20178 yr TE knows what the data is. --done TE changes and sends a packet with the changed data. --half done Client gets the packet and changes its local copy of the TE. --missing entirely Client re-renders. --done Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable. If you think this is the case, JUST REPORT ME. Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice. Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked. DO NOT PM ME WITH PROBLEMS. No help will be given.
June 25, 20178 yr Author I tried sending the actual energy stored in the TE through the network, but it didn't change much from previous tests.
June 25, 20178 yr Author Done EDIT: I will be gone for a few hours, potentially the entire day. Stay tuned. Edited June 25, 20178 yr by That_Martin_Guy Text is now a link
June 26, 20178 yr Author 8 hours ago, diesieben07 said: To avoid confusion like this you should only have that one, parameterless constructor How would I then get the proper BlockLamp object? Checking if the block at the TE's position is an instance of BlockLamp crashes the game in my tests. 8 hours ago, diesieben07 said: That should now work. Is the packet received on the client? Is getLightValue called on the client and is it computing the correct light value? I added this and this to my classes to log this exact thing. It seems it does update its lighting when its at an energy level of MEDIUM, LOW or EMPTY, but not otherwise. Here is an excerpt from the resulting log with some of my own comments.
June 26, 20178 yr 32 minutes ago, That_Martin_Guy said: How would I then get the proper BlockLamp object? You have to create a setter for it. Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable. If you think this is the case, JUST REPORT ME. Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice. Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked. DO NOT PM ME WITH PROBLEMS. No help will be given.
June 26, 20178 yr Author This version of the TE results in this crash whenever I join a world with a lamp in it or place a new one down.
June 26, 20178 yr Author The best way I could figure out on my own was to implement storage like this public TileEntityLamp() { storage = new EnergyStorage(Integer.MIN_VALUE); PLAYER_ENTRY_LIST.setAccessible(true); } and change update to this public void update() { if (worldObj.getBlockState(this.pos).getBlock() instanceof BlockLamp) { BlockLamp lamp = (BlockLamp) worldObj.getBlockState(pos).getBlock(); if(lifetime == 0) lifetime = lamp.maxLifetime; if(storage.getMaxEnergyStored() == Integer.MIN_VALUE) storage = new EnergyStorage(lamp.capacity); if (!worldObj.isRemote) { EnergyLevel energyLevel = EnergyLevel.getEnergyLevel(storage.getEnergyStored(), storage.getMaxEnergyStored()); storage.modifyEnergyStored(-lamp.loss); if (EnergyLevel.getEnergyLevel(storage.getEnergyStored(), storage.getMaxEnergyStored()) != energyLevel) { switch (EnergyLevel.getEnergyLevel(storage.getEnergyStored(), storage.getMaxEnergyStored())) { case EMPTY: break; case LOW: lifetime -= 1 * lamp.lowEnergyMultiplier; break; case MEDIUM: lifetime -= 1 * lamp.mediumEnergyMultiplier; break; case HIGH: lifetime -= 1 * lamp.highEnergyMultiplier; break; } try { List<EntityPlayerMP> playerList = (List<EntityPlayerMP>) PLAYER_ENTRY_LIST.get(((WorldServer) worldObj).getPlayerChunkMap().getEntry(worldObj.getChunkFromBlockCoords(pos).xPosition, worldObj.getChunkFromBlockCoords(pos).zPosition)); LampEnergyMessage message = new LampEnergyMessage(storage.getEnergyStored(), pos); for (EntityPlayerMP player : playerList) { BrightenUp.network.sendTo(message, player); } } catch (IllegalAccessException e) { e.printStackTrace(); } worldObj.checkLight(pos); worldObj.notifyBlockUpdate(pos, worldObj.getBlockState(pos), worldObj.getBlockState(pos), 3); this.markDirty(); } } } } . This would not work in normal code since it resets every time you log in, but it was the closest solution I could find. Even after this, though, it has the exact same problem as before. Doesn't update when recieving power. When the creative energy cell is broken it stays unlit, then goes to medium, then low, then empty.
June 26, 20178 yr From what I can see here you are only sending a packet to the player if this condition is correct: 41 minutes ago, That_Martin_Guy said: EnergyLevel.getEnergyLevel(storage.getEnergyStored(), storage.getMaxEnergyStored()) != energyLevel the getter is fine, but the energyLevel is not. You set it 2 lines above as 41 minutes ago, That_Martin_Guy said: EnergyLevel energyLevel = EnergyLevel.getEnergyLevel(storage.getEnergyStored(), storage.getMaxEnergyStored()); and then you reduce the energy level by loss of energy. If something modifies the energy level of your tile you will not have that change handled here. This way of handling energy changes only handles energy loss, not gain and that is what you are observing. You need to figure out a different way of handling energy changes, maybe have something like energyLevelLastTick as a field in your TE, have that set at the end of your update and compare against it at the beginning. 44 minutes ago, That_Martin_Guy said: This would not work in normal code since it resets every time you log in As long as you read and write your energy storage to TE's NBT data it won't reset. What makes you think that it resets?
June 26, 20178 yr Author 1 minute ago, V0idWa1k3r said: maybe have something like energyLevelLastTick as a field in your TE Great idea! Don't know why I couldn't figure that out myself. I will post the result when I'm done. 2 minutes ago, V0idWa1k3r said: What makes you think that it resets? storage = new EnergyStorage(Integer.MIN_VALUE); in the constructor. If it gets called when I re-enter the world it will reset the storage variable, won't it?
June 26, 20178 yr Just now, That_Martin_Guy said: in the constructor. If it gets called when I re-enter the world it will reset the storage variable, won't it? Yes it will. And the next thing it will do is load your TE from NBT data, which contains your storage data aswell. In the end you will end up with the new storage object being identical to the one you had when leaving the world.
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.