Johni0702 / BetterPortals

A Minecraft Mod which adds see-through portals usable without any loading screen
GNU General Public License v3.0
106 stars 15 forks source link

Crashes with Stellar Sky mod installed #85

Open Johni0702 opened 5 years ago

Johni0702 commented 5 years ago

someone said on CF

Warrof commented 5 years ago

Definitely. Almost instant crash with Stellar Sky installed. (Game lags out for about 10 seconds before crash occurs) crash-2019-07-13_19.37.37-client.txt

SamboyCoding commented 5 years ago

I noticed you said in #120 that you were waiting for StellarSky to release the version currently in dev (that potentially wouldn't crash), but no actual work has been done on the mod since 17 July 2018 - over a year ago. I'd really like to use these mods together if at all possible, but I'm not holding out for a fix from their side, unfortunately.

Johni0702 commented 5 years ago

I'd really like to use these mods together if at all possible, but I'm not holding out for a fix from their side, unfortunately.

Judging from the crash report, the cause of this issue is StellarSky (at least the most recent release of it) assuming there to be only one world, so this will almost certainly have to be fixed on their side.

Tank-Missile commented 4 years ago

I just thought I would pop in here since Stellar Sky development is pretty dead at the moment. I have a pull request here that does fix the mod crashing, but there's an issue where the sky model through the portal does not load until the player goes through the portal. This is because Stellar Sky uses player respawns and dimension changes to load the sky model for that player. The only way that I have found to detect a change without going through the portal is onWorldLoad, but a player is obviously not associated with this event. I need a way to load the sky model for a player or players that are within range of the portal but have not stepped into it. Perhaps onPlayerJoinDimension could be fired for players around the portal when it's activated? The problem with this solution is that the player has not actually joined the dimension, but simply loaded it from another. I also need to consider in a multiplayer environment players that are not near the portal when it's activated, but go near it once it is.

Johni0702 commented 4 years ago

@Tank-Missile Could you elaborate on what you mean by "loading the sky model for a player"? Do you need to send packets from the server to the client or do you need to prepare some GL resources (in which case, what do you need the player entity for?) or is it something else?

Depending on what you need to do, you can probably use the EntityJoinWorldEvent on the server or the WorldEvent.Load event on the client. BP works by spawning fake players on the server for each world which a client can see and then tunnels any packets sent to that fake player via the real player's connection to the client where it swaps out mc.world, mc.player, etc. before letting the normal packet code handle them. The only thing to be aware of is that the fake and the real player can swap worlds (e.g. when the real one passes through a portal).

Tank-Missile commented 4 years ago

@Johni0702 To be honest, I had to fiddle around in the dark quite a bit to figure out what was wrong, but if it wasn't for your hint of the mod crashing when more than one world was loaded at once, I probably would had much more difficultly finding a solution. Due to lack of documentation, from what I pieced together the SkyModel is a culmination of rendering the skybox and the position of the objects in the sky such as the moon, stars, etc. In layman terms, it's just the sky. However, Stellar Sky creates only 1 SkyModel object when the mod is first loaded. I came up with a simple analogy to explain the issue. I call it the One Room Hotel Conflict.

Imagine there is only one hotel room in a hotel, and a guest comes to occupy the room. Under normal circumstances, if a new guest arrives, the current guest leaves. However, under strange new circumstances, the current guest refuses to leave. When a new guest arrives and attempts to enter the occupied room, a conflict occurs between the two guests. Now replace "hotel room" with "SkyModel" and "guest" with "data". This "strange new circumstance" is your mod.

What I'm trying to say is because more than 1 world is loaded at once due to your mod, the 1 SkyModel object created at runtime gets filled with incorrect data causing a null pointer exception.

I'm no expert on the internals of Minecraft, but I do know the sky is rendered client side. I assume on a server, a new world loading and a player joining a world is handled server side.

Johni0702 commented 4 years ago

SkyModel is a culmination of rendering the skybox

So, it's a client-side thing?

In that case, I don't get what's missing. Looking at your PR, you already seem to be using the WorldEvent.Load/Unload events to create / tear down SkyModel objects.

I need a way to load the sky model for a player or players that are within range of the portal but have not stepped into it.

WorldEvent.Load/Unload events do precisely that on the client (well, except also for the main world, but that should be what you want as far as I understand).

Johni0702 commented 4 years ago

I'm no expert on the internals of Minecraft, but I do know the sky is rendered client side. I assume on a server, a new world loading and a player joining a world is handled server side.

To clarify, WorldEvent.Load is fired whenever a new World object is constructed. That happens for both, ServerWorld and ClientWorld. The former because a new world was loaded/created on the server, the latter because a world was synced to the client (i.e. it's new to the client, e.g. because they entered the syncing range of a portal, or they just joined the server, or they respawned, or changed worlds in a different way). Unload events are analogous. The only conceptual thing which BP changes is that without it, on the client there's always an Unload event before the next Load event, whereas with BP it's totally possible that there are e.g. two Loads and only significantly later the two Unloads (and those may also be in arbitrary order).

Tank-Missile commented 4 years ago

@Johni0702 It's only used in the ClientProxy class, so I assume so. I do use WorldEvent.load/Unload to handle SkyModel creation and removal. Because I'm using a HashMap of SkyModels, conflicts can never occur between them. However, the problem I'm running into is even though they are being created on world load, the SkyModel that was created is not filled with data until the player steps through the portal into the newly loaded world. When a SkyModel is created, 3 methods have to be fired for it to work: setupStellarLoad, setupDimensionLoad, and setupSkyRenderer. If all 3 of these methods do not fire when the SkyModel is created, the SkyModel will not function correctly.

Johni0702 commented 4 years ago

setupStellarLoad, setupDimensionLoad, and setupSkyRenderer

So, where are they supposed to be called from? Or rather, why can't they just all be called in WorldEvent.Load?

Tank-Missile commented 4 years ago

The methods originate from the ClientProxy class. They're called from the StellarAPINetworkManager class in StellarAPI.

Johni0702 commented 4 years ago

Ah, so it is packets which need to be sent. In that case

use the EntityJoinWorldEvent

check if the entity is a EntityPlayerMP and if so, call onSync for it.

Tank-Missile commented 4 years ago

Is that event called even if the player has not actually joined the world? The portal is loaded before the player goes through it. I'm assuming this has to do with the fake player being spawned having his view tunneled to the player or other players?

Johni0702 commented 4 years ago

Is that event called even if the player has not actually joined the world? The portal is loaded before the player goes through it. I'm assuming this has to do with the fake player being spawned having his view tunneled to the player or other players?

It is fired for the fake players just like as if a player joined that world in spectator mode.

Ideally third-party mods shouldn't need to be aware of BP (except the fact that there may be multiple worlds on the client), they can treat the views just like spectator players (cause that's what they technically are). EntityJoinWorldEvent fires when any entity (or player) joins a world, so any mod needing to sync per-world data to players should just send its packets to any players joining that world. The only reason it doesn't work with StellarSky is that it makes assumptions about how exactly players will join the world (namely via logging in, changing dimensions or respawning, i.e. PlayerLoggedInEvent, PlayerChangedDimensionEvent or PlayerRespawnEvent) and fake players really don't do any one of those (but they do join the world, i.e. EntityJoinWorldEvent).

Tank-Missile commented 3 years ago

It's been a while, but doing what you suggested fixed the issue of the sky model not updating. I've created a pull request here for those who are interested. I have jar releases available on the forked repos for now.