AbyssalCraft

AbyssalCraft

20M Downloads

Sacroth Missing !this.world.isRemote Before Setting Nightime

jredfox opened this issue ยท 9 comments

commented

Sacroth Missing !this.world.isRemote Before Setting Nightime onInitialSpawn. This causes issues with anything trying to render it on client side making the time equal to night then flashing back to what the server says it is as it should be. So please fix your check there. It also would mess up on client side a few ticks to by simply spawning one in initially when it is daytime. This isn't silkspawners job to correct night/daytime nor re-add any broken blocks bosses would do because any data manipulation should be done on server side

commented

Judging by this report on the Mojang bug tracker, I shouldn't have to check sides in that method, as it's meant to be server side only (but MobSpawnerBaseLogic improperly calls it on the client side). Seeing as your mod overrides MobSpawnerBaseLogic, wouldn't it be more appropriate if you removed the part where it invokes EntityLivingBase#onInitialSpawn on the client side, rather than every mob-adding mod that might do something in that method putting side checks (you might wanna use some ASM to patch the vanilla mobs as well if you choose not to fix the vanilla bug in your version of MobSpawnerBaseLogic)?

I would question why someone would even put a boss mob in a mob spawner, but there's nothing stopping someone from doing such a thing, so that question would cancel itself out.

commented

I call EntityLivingBase#onInitialSpawn so it accurately renders the mob accurately So skeletons have their bows, like NEI and older mob spawners from like 1.7.10 did. You should have to check if world is remote because it is a two sided event. On Client side it constantly switches between day and night so just insert the !world.isRemote and it will fix it.

it's also problematic code if the entity isn't synced with the exact same tick when it spawns it will show a very specific time and will be a couple ticks off the server side packet

commented

Since it's AbyssalCraft, having the rapid day/night sounds like a feature to me.

commented

issue is not invalid the client side onInitialSpawn can and will fire by both me and other mods on client side this issue is on your side not other mods. Check if world is not remote before applying server only data. You don't create an explosion on client side or set blocks on client side so you don't certainly set the time on client side

commented

It is not only called on the server side many client rendering calls it to give entities their armor wepons etc... Ignorance on not fixing an issue on their side by claiming everything is on every body else's side without investigating can this be called on client side and should server side only data function without a is server side check. In this case you comment without reading you don't investia get can this get called on client side.

Vanilla mob spawners I am pretty sure at one point without any mods did in fact have onInitialSpawn done on client side there is no significate change on why it's a server only function in matter of fact I am sure there are plenty of other mods that do render client side entities with calling this one time at least
with calling onInitialSpawn
And it's not their fault that another mod breaks simple things.

I did investiage this. Mob spawners do in fact call on initial spawn when spawning the entity into the world. So calling it on client side works with literally displaying a possiblity of what it should be spawning. Every other mod then it has to be on this mods side not vanilla's/silkspawners. And as stated above some mods and even vanilla at one point did in fact call said method.

This is my last comment on here anybody disagreeing with valid issues out of ignorace shouldn't be coding minecraft content mods or java to begin with and it will be broken not on other mods side but, on yours. For example take a look at jei ram leaks of 2G+ because they cache everything instead of per page for no apperent reason. You want to be like jei have 100 valid issues closed and blame everyone else keep it up. You will also get fired from real world coding jobs and can't keep a job for this crap.

commented

@jredfox The contract of the EntityLiving#onInitialSpawn method is that it is only called on the server. This is well established and every other mod upholds this contract. MC-114019 represents a bug with Minecraft violating this very contract (evident by vanilla implementations not checking the world's sidedness -- they expect it to only be called on the server-side).

It is not the responsibility of other mod developers to workaround this error on Mojang's part. If you are writing your own mod and choosing to follow this broken behaviour, then your mod itself is broken.

commented

You will also get fired from real world coding jobs and can't keep a job for this crap.

Funnily enough I am in the process of quitting a real world coding job, but the reason for that isn't because I refuse to follow broken coding conventions (it's more in line with depression and other things related to that).
Anyways, since the conversation in this issue is completely one-sided, and destructive, I'm locking this issue to prevent that from escalating even more.

commented

Go ahead and launch vanilla minecraft mob spawners 1.12.2 They have the skeleton equipment enabled and called EntityLivingBase#onInitialSpawn proof this is not server side only without any other mods. Also proves all previous comments were not checked and out of ignorance. This also proves that you need a server side check for data manipulation that should only happen on the server

steps to reproduce:
/setblock ~ ~ ~ mob_spawner 0 replace {SpawnData:{id:skeleton}}
2019-05-15_22 55 35

commented

@Shinoow you may want to lock this issue