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

Entity Update #197

Merged
merged 11 commits into from Feb 19, 2016
Merged

Entity Update #197

merged 11 commits into from Feb 19, 2016

Conversation

LukBukkit
Copy link
Contributor

  • Fixed bug when saving villagers and wolves
  • Living entities have now a real death animation
  • Many living entities now send a metadata package
  • If its possible the metadatas of the entities, were saved in the metadata list of the entity class.
  • Entities now sending their health to the player
  • Entities have now a correct max health

@LukBukkit LukBukkit changed the title Entity Update [WIP]Entity Update Feb 13, 2016
@LukBukkit LukBukkit changed the title [WIP]Entity Update [WIP] Entity Update Feb 13, 2016
@@ -117,6 +117,8 @@ public void setScaleForAge(boolean isAdult) {
return messages;
}



Copy link
Contributor

Choose a reason for hiding this comment

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

just to nitpick, is this diff necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorrry my IDE made this. It will be removed in the next commit!

-Fixed bug when saving villagers and wolves
-Living entities have now a real death animation
-Many living entities now send a metadata package
-If its possible the metadatas of the entities, were saved in the metadata list of the entity class.
-Entities now sending their health to the player
-Entities have now a correct max health
- Sorted and add * imports
- Removed unused update messages
@@ -27,7 +27,7 @@
/**
* Creates a new ageable monster.
* @param location The location of the monster.
* @param type The type of monster.
* @param type The type of monster.
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary spacing.

@mastercoms
Copy link
Member

I think GlowLivingEntity should have a new constructor for max health instead of using setmaxhealthandhealth.

The max health is now set using the constructor
@LukBukkit LukBukkit changed the title [WIP] Entity Update Entity Update Feb 13, 2016

@Override
public List<Message> createUpdateMessage() {
return super.createUpdateMessage();
Copy link
Member

Choose a reason for hiding this comment

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

I think that is needless

@mastercoms
Copy link
Member

Now, I think that there should be a EntityRegistry or something, like ItemTable, which can be used like this:

EntityRegistry.instance.getMaxHealth(GlowWolf.class);

I would be ok if you would not like to add it in this PR, as it slightly extends the scope of the PR.

Of course, we wouldn't remove the constructor with max health even with EntityRegistry, as plugins may want to spawn in entities with custom max health. Or perhaps, we could allow plugins access to EntityRegistry so they can define their own entities or new properties for vanilla entities.

@LukBukkit
Copy link
Contributor Author

@mastercoms I will do this in one of my next PR's.

public void setTamed(boolean isTamed) {
metadata.setBit(MetadataIndex.WOLF_FLAGS, TameableFlags.IS_TAME, isTamed);
if (tamed != isTamed) {
//Change max health of wolf when he's got tamed. See MinecraftWiki for more information!
Copy link
Member

Choose a reason for hiding this comment

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

Please add a space between // and the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will do this tomorrow.

@mastercoms
Copy link
Member

Looks good to me.


for (Objective objective: getServer().getScoreboardManager().getMainScoreboard().getObjectivesByCriteria(Criterias.HEALTH)) {
for (Objective objective : getServer().getScoreboardManager().getMainScoreboard().getObjectivesByCriteria(Criterias.HEALTH)) {
objective.getScore(this.getName()).setScore((int) health);
}

if (health == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Less than or equal to. Just in case.

@mastercoms
Copy link
Member

There is an error when spawning ocelots:

[SEVERE] Unable to spawn entity: 
java.lang.reflect.InvocationTargetException
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
    at net.glowstone.block.itemtype.ItemSpawn.rightClickBlock(ItemSpawn.java:29)
    at net.glowstone.net.handler.play.player.BlockPlacementHandler.handle(BlockPlacementHandler.java:120)
    at net.glowstone.net.handler.play.player.BlockPlacementHandler.handle(BlockPlacementHandler.java:23)
    at net.glowstone.libs.com.flowpowered.networking.session.BasicSession.handleMessage(BasicSession.java:80)
    at net.glowstone.libs.com.flowpowered.networking.session.BasicSession.messageReceived(BasicSession.java:139)
    at net.glowstone.net.GlowSession.pulse(GlowSession.java:429)
    at net.glowstone.net.SessionRegistry.pulse(SessionRegistry.java:23)
    at net.glowstone.scheduler.GlowScheduler.pulse(GlowScheduler.java:173)
    at net.glowstone.scheduler.GlowScheduler.access$100(GlowScheduler.java:24)
    at net.glowstone.scheduler.GlowScheduler$2.run(GlowScheduler.java:110)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
    at net.glowstone.entity.passive.GlowOcelot.setCatType(GlowOcelot.java:25)
    at net.glowstone.entity.passive.GlowOcelot.<init>(GlowOcelot.java:15)
    ... 21 more

And another when saving a world with an enderman:

[WARNING] Error saving GlowEnderman in GlowChunk{world=world,x=16,z=-3}
java.lang.IllegalArgumentException: Unknown entity type to save: class net.glowstone.entity.monster.GlowEnderman
    at net.glowstone.io.entity.EntityStorage.find(EntityStorage.java:142)
    at net.glowstone.io.entity.EntityStorage.save(EntityStorage.java:162)
    at net.glowstone.io.anvil.AnvilChunkIoService.write(AnvilChunkIoService.java:212)
    at net.glowstone.ChunkManager.performSave(ChunkManager.java:379)
    at net.glowstone.GlowWorld.lambda$save$0(GlowWorld.java:980)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at net.glowstone.scheduler.GlowTask.run(GlowTask.java:163)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

And another save error for ocelot:

[WARNING] Error saving GlowOcelot in GlowChunk{world=world,x=16,z=-3}
java.lang.NullPointerException
    at net.glowstone.io.entity.OcelotStore.save(OcelotStore.java:22)
    at net.glowstone.io.entity.OcelotStore.save(OcelotStore.java:7)
    at net.glowstone.io.entity.EntityStorage.save(EntityStorage.java:165)
    at net.glowstone.io.anvil.AnvilChunkIoService.write(AnvilChunkIoService.java:212)
    at net.glowstone.ChunkManager.performSave(ChunkManager.java:379)
    at net.glowstone.GlowWorld.lambda$save$0(GlowWorld.java:980)
    at net.glowstone.GlowWorld.maybeAsync(GlowWorld.java:1664)
    at net.glowstone.GlowWorld.save(GlowWorld.java:978)
    at net.glowstone.GlowWorld.save(GlowWorld.java:968)
    at org.bukkit.command.defaults.SaveCommand.execute(SaveCommand.java:30)
    at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:185)
    at net.glowstone.GlowServer.dispatchCommand(GlowServer.java:1165)
    at net.glowstone.ConsoleManager$CommandTask.run(ConsoleManager.java:214)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at net.glowstone.scheduler.GlowTask.run(GlowTask.java:163)
    at net.glowstone.scheduler.GlowScheduler.pulse(GlowScheduler.java:181)
    at net.glowstone.scheduler.GlowScheduler.access$100(GlowScheduler.java:24)
    at net.glowstone.scheduler.GlowScheduler$2.run(GlowScheduler.java:110)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

@mastercoms
Copy link
Member

After these issues are fixed, I will test reading this saved data using the vanilla Minecraft server.

@LukBukkit
Copy link
Contributor Author

I currently save nothing, I just send these packets. So I think the Minecraft server won't has got so much too read. If you want I can implement this today too. And I don't why the enderman isn't correctly this bug appears for me sometimes, but is works sometimes too. Then I'll try To fix it this afternoon. :)

@mastercoms
Copy link
Member

Ah, I thought with the save error, there was something being saved. I think saving has already been done in another commit, and this extra data on the entities is confusing it.

@LukBukkit
Copy link
Contributor Author

The cat errors are partly mine, but I didn't edit anything realting the enderman.

@LukBukkit
Copy link
Contributor Author

I fixed the error in connection with the cat, now I try to fix the enderman error. FIXED IT
The enderman storage just wasn't registered and the NBT ID of the Wolf was wrong. See WolfStorage.

- Cleanup ( Added spaces after comments and changed two if queries ).
- Removed useless createUpdateMessage.
- Fixed Ocelot bug on saving and loading.
- Added EndermanStore to EntityStorage. I don't know why it wasn't there.
- Fixed Wolf bug on saving and loading. Wolves aren't ozelots.
@kamcio96
Copy link
Member

isAwake in GlowBat can be stored in metadata? like isClimbing in GlowSpder

@mastercoms
Copy link
Member

There is an error when loading a world with entities spawned in vanilla. I believe it is because your isPowered, isVillager, and etc should first check if the tag exists, and if it does not, return 0, or whatever and if it does exist, return the value of the tag. Here is the error (there were variations of this error for SkeletonType and IsVillager, so it should only be necessary to change those 3 getters, but I will trust your judgement on whether to change all of them or not):

[WARNING] Error loading entity in GlowChunk{world=world,x=-11,z=0}: Creeper
java.lang.IllegalArgumentException: Compound does not contain ByteTag "powered"
    at net.glowstone.util.nbt.CompoundTag.getTag(CompoundTag.java:266)
    at net.glowstone.util.nbt.CompoundTag.get(CompoundTag.java:260)
    at net.glowstone.util.nbt.CompoundTag.getBool(CompoundTag.java:63)
    at net.glowstone.io.entity.CreeperStore.load(CreeperStore.java:15)
    at net.glowstone.io.entity.CreeperStore.load(CreeperStore.java:6)
    at net.glowstone.io.entity.EntityStorage.createEntity(EntityStorage.java:132)
    at net.glowstone.io.entity.EntityStorage.loadEntity(EntityStorage.java:124)
    at net.glowstone.io.anvil.AnvilChunkIoService.read(AnvilChunkIoService.java:108)
    at net.glowstone.ChunkManager.loadChunk(ChunkManager.java:132)
    at net.glowstone.ChunkManager.populateChunk(ChunkManager.java:199)
    at net.glowstone.ChunkManager.forcePopulation(ChunkManager.java:231)
    at net.glowstone.GlowWorld.setKeepSpawnInMemory(GlowWorld.java:792)
    at net.glowstone.GlowWorld.<init>(GlowWorld.java:338)
    at net.glowstone.GlowServer.createWorld(GlowServer.java:1449)
    at net.glowstone.GlowServer.start(GlowServer.java:501)
    at net.glowstone.GlowServer.run(GlowServer.java:134)
    at net.glowstone.GlowServer.main(GlowServer.java:109)

Also, this error was thrown when loading a player built golem from the vanilla world:

[WARNING] Error loading entity in GlowChunk{world=world,x=-4,z=0}: VillagerGolem
java.lang.IllegalArgumentException: Cannot assign true to GOLEM_PLAYER_BUILT, expects BYTE
    at net.glowstone.entity.meta.MetadataMap.set(MetadataMap.java:52)
    at net.glowstone.entity.monster.GlowIronGolem.setPlayerCreated(GlowIronGolem.java:26)
    at net.glowstone.io.entity.IronGolemStore.load(IronGolemStore.java:15)
    at net.glowstone.io.entity.IronGolemStore.load(IronGolemStore.java:6)
    at net.glowstone.io.entity.EntityStorage.createEntity(EntityStorage.java:132)
    at net.glowstone.io.entity.EntityStorage.loadEntity(EntityStorage.java:124)
    at net.glowstone.io.anvil.AnvilChunkIoService.read(AnvilChunkIoService.java:108)
    at net.glowstone.ChunkManager.loadChunk(ChunkManager.java:132)
    at net.glowstone.ChunkManager.populateChunk(ChunkManager.java:199)
    at net.glowstone.ChunkManager.forcePopulation(ChunkManager.java:231)
    at net.glowstone.GlowWorld.setKeepSpawnInMemory(GlowWorld.java:792)
    at net.glowstone.GlowWorld.<init>(GlowWorld.java:338)
    at net.glowstone.GlowServer.createWorld(GlowServer.java:1449)
    at net.glowstone.GlowServer.start(GlowServer.java:501)
    at net.glowstone.GlowServer.run(GlowServer.java:134)
    at net.glowstone.GlowServer.main(GlowServer.java:109)

I believe it uses a byte for some reason, 1 for true, and 0 for false.

I check now, if a tag exits, before I store it into the entity metadata. When a tag doesn't exits I store a default value into the entity class.
@LukBukkit
Copy link
Contributor Author

Ups forgot to format code. Sorry, but the errors are should fixed now :)

@mastercoms
Copy link
Member

Type of playerbuilt for golem is still boolean, it should be byte.

@LukBukkit
Copy link
Contributor Author

The method I calling translates the boolean into a byte / translates a byte into a boolean. So I think it should work. See here and here

@mastercoms
Copy link
Member

ah, ok

@mastercoms
Copy link
Member

Still getting the same error as before with the iron golem:

[WARNING] Error loading entity in GlowChunk{world=world,x=14,z=13}: VillagerGolem
java.lang.IllegalArgumentException: Cannot assign false to GOLEM_PLAYER_BUILT, expects BYTE
    at net.glowstone.entity.meta.MetadataMap.set(MetadataMap.java:52)
    at net.glowstone.entity.monster.GlowIronGolem.setPlayerCreated(GlowIronGolem.java:26)
    at net.glowstone.io.entity.IronGolemStore.load(IronGolemStore.java:16)
    at net.glowstone.io.entity.IronGolemStore.load(IronGolemStore.java:6)
    at net.glowstone.io.entity.EntityStorage.createEntity(EntityStorage.java:132)
    at net.glowstone.io.entity.EntityStorage.loadEntity(EntityStorage.java:124)
    at net.glowstone.io.anvil.AnvilChunkIoService.read(AnvilChunkIoService.java:108)
    at net.glowstone.ChunkManager.loadChunk(ChunkManager.java:132)
    at net.glowstone.ChunkManager.populateChunk(ChunkManager.java:199)
    at net.glowstone.ChunkManager.forcePopulation(ChunkManager.java:231)
    at net.glowstone.GlowWorld.setKeepSpawnInMemory(GlowWorld.java:792)
    at net.glowstone.GlowWorld.<init>(GlowWorld.java:338)
    at net.glowstone.GlowServer.createWorld(GlowServer.java:1449)
    at net.glowstone.GlowServer.start(GlowServer.java:501)
    at net.glowstone.GlowServer.run(GlowServer.java:134)
    at net.glowstone.GlowServer.main(GlowServer.java:109)

You're using MetaData.set, which checks for the type of the value you pass in, which in this case, you are passing byte. It detects that this type is wrong, and throws an error.

@gdude2002 gdude2002 added the wip label Feb 19, 2016
@mastercoms mastercoms removed the wip label Feb 19, 2016
mastercoms added a commit that referenced this pull request Feb 19, 2016
@mastercoms mastercoms merged commit 4a68c25 into GlowstoneMC:master Feb 19, 2016
@mastercoms
Copy link
Member

Thank you for this awesome contribution!

@kamcio96
Copy link
Member

okey, so now ai 😄

@mastercoms
Copy link
Member

Entities branch has some awful progress on that.

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

Successfully merging this pull request may close these issues.

None yet

6 participants