Varied crashing with ticking entity.

SteamMonkey commented 7 years ago

[x] I am running the latest mod versions of MCA and RadixCore.
[x] I can reproduce this issue with just MCA and RadixCore installed.
[x] I can provide my Minecraft version and MCA version.
[x] I have searched for the issue previously and it was either not previously reported, or previously fixed and I'm having the same problem.
[] I am crashing and can provide my crash report.

Versions 1.10.2 - Latest MCA on Curse.

Multiple crashes on various things. water falls on agricraft crops. Mechanical users in Entity mode, etc. Basically multiple "Ticking entity" updates will cause an immediate crash, kill single player or servers until MCA is disabled.

Reproduce Steps

Plant agricraft crop in crop sticks, spill water on top. Crash. Place mechanical user, put any item in side, set it to entity. Stand on top of the mechanical user, walk in front of it, or have another entity walk in front of it. Crash.

Additional Information

Jo-Mamma75 commented 7 years ago

Also just had this crash with Agricraft crops and a creeper explosion without running water.

Jo_Mamma75 Iskall85 Community Administrator FoolCraft Dev Team

WildBamaBoy commented 7 years ago

Can either of you provide the crash report for this issue?

SteamMonkey commented 7 years ago

Mechanical User: Entity mode, clicking on a player. https://pastebin.com/LW5Uk3B5

Jo-Mamma75 commented 7 years ago

Agricraft Crash: Flowing water on crops. https://pastebin.com/v8JrCsdU

Virtuoel commented 7 years ago

From what I can tell, the Mechanical User crash appears to happen because the "else if" condition in the entityInteractEventHandler in EventHooksForge is hardcoded to check against a player whose name contains "[CoFH]" and not against any FakePlayer.

That Agricraft crash appears to be unrelated to MCA from the report.

Jo-Mamma75 commented 7 years ago

@Virtuoel I would be inclined to agree with you on the second comment if it weren't for the fact that I can load up a 1.10.2 instance with only Agricraft and MCA, produce the same set of conditions and results, remove MCA and then not have the problem.

As for the first comment, what would you suggest as the solution?

Virtuoel commented 7 years ago

@Jo-Mamma75 In that case, I haven't got a clue what causes the Agricraft issue.

To solve the FakePlayer issue, one would simply need to add "!(event.getEntityPlayer() instanceof FakePlayer)" to the list of conditions, and optionally remove the "[CoFH]" check. It might also be a good idea to add a condition like this to other places where things involving Players occur that should not be applied to FakePlayers.

SteamMonkey commented 7 years ago

From what I can tell, mca checks entities when they are created and then changes villagers to mca villagers. Is there a chance it could be seeing any entity at creation and conflicting with certain things? That could cause the agricraft bug if the entity creation from the drop isn't normal.

Sent from mobile, please ignore autocorrects.

On Apr 12, 2017, at 10:19 PM, Virtuoel notifications@github.com wrote:

@Jo-Mamma75 In that case, I haven't got a clue what causes the Agricraft issue.

To solve the FakePlayer issue, one would simply need to add "!event.getEntityPlayer() instanceof FakePlayer" to the list of conditions, and optionally remove the "[CoFH]" check. It might also be a good idea to add a check like this to other places where events involving players occur.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

Jo-Mamma75 commented 7 years ago

@WildBamaBoy Any ideas on this issue?

SteamMonkey commented 7 years ago

So, in regards to the Mechanical user issue, your recommendation is for Forge to be changed?

As far as I can tell, with MCA disabled, the mechanical user crash doesn't occur. With MCA enabled, it does. Given that situation, perhaps theres a solution that involves something within MCA and not within forge that we could come up with?

Right clicking on a player with MCA enabled brings up a 'new' feature, the whole marriage thing. That process is most likely what's causing the crash if I had to guess. Fake users are most likely unable to process the MCA even that occurs when a player right clicks, but they are still triggering the event. Would it be possible to make a change to that event so that it only triggers when a legitimate player right clicks another player? Dollars to donuts, that'll resolve the crash without having to update Forge's code which can have a LOT broader impact. It may even resolve some of the other random crashes that are occurring with other mods as well. Nothing should be able to initiate that marriage GUI except an actual player anyway, so locking it to specifically "player" seems like a good practice in the first place.

Especially with the rest of the COFH mod lines hitting 1.10 soon, you're going to see a lot more "fake player" action in a lot more mod packs.

Virtuoel commented 7 years ago

@SteamMonkey No, for the Mechanical User issue, there are no Forge changes needed. Minecraft Comes Alive needs to change it's entityInteractEventHandler method in its EventHooksForge class. Nothing to do with Forge, just that the MCA class happens to have "Forge" in the name.

The current code checks against an Autonomous Activator right clicking the player, and doesn't check against other types of machines. Adding the condition of !(event.getEntityPlayer() instanceof FakePlayer) should work for most types of right clicking machines that simulate Players.

WildBamaBoy / minecraft-comes-alive