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

Incompatible with Supplementaries 1.20-3.1.15 #1107

Open
1 task done
Barerock opened this issue Feb 19, 2025 · 29 comments
Open
1 task done

Incompatible with Supplementaries 1.20-3.1.15 #1107

Barerock opened this issue Feb 19, 2025 · 29 comments
Labels
bug Something isn't working

Comments

@Barerock
Copy link

This issue occurs when only Valkyrien Skies and addons are installed and no other mods

  • I have tested this issue and it occurs when no other mods are installed

Minecraft Version

1.20.x

Mod Loader

Forge

Issue description

MehVahdJukaar/Supplementaries#1377

Issue reproduction

See linked

Logs

https://github.com/user-attachments/files/18875904/latest.log

@Barerock Barerock added the bug Something isn't working label Feb 19, 2025
@LenKagamine
Copy link

Oh my gosh, thank you. I was wondering what was causing the crashes.

@getItemFromBlock
Copy link

getItemFromBlock commented Feb 20, 2025

Known issue, this is caused by supplementaries registering their dimension at the wrong time (so not caused by VS).
The best you can do is downgrade supplementaries below version 3.0 until their dev fixes it.

@Barerock
Copy link
Author

That's what Brickyboy and I surmised might be happening. Good to know, you should talk to MehVahdJukaar about it, since they seem to think it's on VS's end.

@getItemFromBlock
Copy link

I am not responsible for what they do, if they think they can blame everything on other mod's fault then we can't really do anything about it

@getItemFromBlock
Copy link

I know they have a discord server, and someone already talked about this issue a little while ago
Maybe something changed here?

@Barerock
Copy link
Author

I thought that was the cannon issue, which I never experienced? I meant that they might be willing to look at it further since you know why exactly this issue is happening and that it's not VS causing it.

@getItemFromBlock
Copy link

It might be something else that they are doing, the error seems to be caused by the same problem as the cannon crash (registering a dimension at the wrong tick time). Maybe they updated their mod and changed something in relation to that?

@Barerock
Copy link
Author

Perhaps. Again, you guys should communicate. I hear it helps sort issues.

@getItemFromBlock
Copy link

From what I see communication was attempted on their discord server, but without any success
Sorry but I can't really do more, if they don't want to fix it I can't force them to do it

@MehVahdJukaar
Copy link

Known issue, this is caused by supplementaries registering their dimension at the wrong time (so not caused by VS). The best you can do is downgrade supplementaries below version 3.0 until their dev fixes it.

So supplementaries doesn't have a dimension. Also what determines what is wrong and good time?

@getItemFromBlock
Copy link

getItemFromBlock commented Feb 20, 2025

As far as I have seen, it uses a "fake level" (which I erroneously called "dimension") in order to handle the cannon shooting. The issue here is that when a new level is registered, Valkyrien Skies tries to add it as a dimension. This should not be an issue by itself, but the fake level is registered in the wrong tick phase which makes valkyrien skies crash.
I am not a developer of VS, but this is what was found out on our side
I will ask for a VS dev to help sort this out

@MehVahdJukaar
Copy link

Well yeah I have a level but not a dimension. Also I don't register anything I just instantiate the object. As for the phase in which it's done it happens on tick event I believe on first game tick, it was delayed in an effort to prevent issues that could happen like this one when mods would make assumptions about levels

@MehVahdJukaar
Copy link

MehVahdJukaar commented Feb 20, 2025

So when you mean tick phase you mean that I should do it in tick event.pre instead of tick event.post? If so, why?

@jcm236
Copy link

jcm236 commented Feb 22, 2025

Well yeah I have a level but not a dimension. Also I don't register anything I just instantiate the object.

VS mixins into the ServerLevel class so it does not need to be registered for it to crash

@jcm236
Copy link

jcm236 commented Feb 22, 2025

So when you mean tick phase you mean that I should do it in tick event.pre instead of tick event.post? If so, why?

vscore's enforced tick order only allows new level instances between servertick.pre and servertick.post

@MehVahdJukaar
Copy link

I don't understand what's this level registering you are talking about. Also my levels lifecycles are independent of server level

@PriestOfFerns
Copy link

@MehVahdJukaar
I think I figured out the issue. It appears to happen when reloading the game with a Faucet block placed. Checking the code:

public void onReloadWithLevel(ServerLevel level) {
        ...

        FakeLevelManager.invalidate(testLevel);
        listeners.forEach(Runnable::run);
    }

Shows that Faucet "invalidates" server level on instance reload. Checking the respective function in moonlight lib:

public static boolean invalidate(Level level) {
      boolean removed = INSTANCES.entrySet().removeIf(e -> e.getValue() == level);

      if (level != null) PlatHelper.invokeLevelUnload(level);
      try {
          if (level instanceof FakeServerLevel) {
              level.close();
          }
      } catch (Exception e) {
          if (PlatHelper.isDev()) {
              throw new RuntimeException(e);
          } else {
              Moonlight.LOGGER.error("An error occurred while closing fake level", e);
          }
      }
      return removed;
  }

Shows that it closes the level it's trying to invalidate. If I had to guess, the server reload event (which faucet subscribes to) happens either independent of tick phase or in a wrong tick phase, causing a level to be closed in the wrong phase, causing VS2 to release an error.

@MehVahdJukaar
Copy link

That's called from ServerStartedEvent

@MehVahdJukaar
Copy link

So is this something that will be fixed? To me the error seems to be the enforcement of something arbitrary like having a level be closed during tick event

@LenKagamine
Copy link

I mean, the issue only started when your mod was updated- Before that, Supplementaries and VS worked fine, so it seems like something in that update broke.

@MehVahdJukaar
Copy link

Yes that updated added all that code. My point is that unless there's a good reason for it, there shouldn't be arbitrary conditions enforced as when dealing with mods you are in a complex environment and you shouldn't make so many assumptions about what other mods might do, for example it would be fine to expect mods not to trow around null in not null able methods as that would mean they are breaking the method contract but for this one there doesn't seem anything that would prohibit it

@jcm236
Copy link

jcm236 commented Feb 23, 2025

Well, vscore does require the tick order to not change. I'm not sure of the specifics though.

@LenKagamine
Copy link

I'm sure if there wasn't a reason for the 'arbitrary conditions', then they wouldn't be there. A mod developer isn't going waste time adding random things to their mod that aren't necessary for it's functions or content.

@MehVahdJukaar
Copy link

Well could have been an oversight or others, not first time I've seen, for example I've seen lithium add a hard crash when a piston like block had an offset different than 0 or 0.5, an assumption that would work in vanilla but falls short in modded as mods can extend classes and have new implementation of them, similar to this case. Fact is that imo if you add arbitrary conditions you should fallback to something else when those fail because mods can and will break (even if very rarely, it's all about edge cases) them as there's nothing in vanilla or forge that enforces them

@LenKagamine
Copy link

Supplementaries seems to have fixed the issue in the latest update. Mods are Supplementaries 3.1.18, Kotlin 4.11.0, Moonlight 2.13.70, and Valkyrien Skies 2.3.0 beta 5.

@Rubydesic
Copy link
Contributor

So is this something that will be fixed? To me the error seems to be the enforcement of something arbitrary like having a level be closed during tick event

I will look into adding some kind of mitigation for this in vs-core but I don't know when I'll have a chance to. Unfortunately, if we didn't hard crash here it could lead to other harder to diagnose errors with our mod.

@Barerock
Copy link
Author

Barerock commented Feb 25, 2025

Supps has not fixed this issue I do not think. I am still crashing on reload. Nor do I have any supplementaries blocks/entities placed. I will do more testing.

Confirmed, backdating to 3.1.13 fixes the issue still. Perhaps this is related more directly to a side mod that uses VS2, rather than VS2 itself. More testing.

@Barerock
Copy link
Author

Barerock commented Feb 25, 2025

No, this is confirmed with VS2 120-2.3.0-beta.5, kotlin-4.11.0-all, supplementaries 1.20-3.1.18, moonlight 1.20-2.13.70 on a brand new world.

Image

To remind everyone, this is not an initial loading crash, this occurs when the user executes /reload or /reloadconfig.

Here is log: latest.log

@LenKagamine
Copy link

I seem to be following the wrong issue then. I was crashing on world load. Oops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants