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

Update to BS v1.25.1 #4

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

TheShadowEevee
Copy link

I went through and updated this mod to 1.25.1; there's a decent amount of changes so I'll try to summarize below. Apologies if this PR description isn't precise, I'm writing this at 3am so I may forget to note something. I'm happy to clarify anything if need be.

General Changes:

  • Updated the compilation method to use CMake primarily, the current suggested way to compile that I saw
  • Fixed/Updated various hooks and methods that had name changes since 1.17.1
  • Changed typings where needed to resolve compilation errors due to changes in methods

This should bring things back up to speed with the latest version generally available to mod, and hopefully make it somewhat easier to update going forward as well. Everything seems to work fine in my testing that was implemented before to my knowledge.

If you (or anyone else) wants to test I made a release on my fork: TheShadowEevee/BeatSaberServerBrowserQuest/releases

@michael-r-elp
Copy link
Member

First of all thanks for updating the mod.

The actual reason I haven't made another release was because I wanted the next release to be fully featured including announcing games to BSSB.
The only thing that held me back from implementing that feature was the lack of a easy to use event handling system, which I now have in MultiplayerCore.
Unfortunately I haven't yet gotten the time to port the code for announcing games.

And with the random liveness crashes some people get with mods like this I wasn't sure if I should be releasing this.

@TheShadowEevee
Copy link
Author

Ah, that makes sense. My main reason for doing this was for personal use, so I may mess with it a little in the way of investigating crashes myself, but I can't promise that. If/When you do find time though to implement the event handling and announcing this PR should hopefully help get that started!

@michael-r-elp
Copy link
Member

After taking a look, I noticed you updated based of the master branch, I already have alot of these code changes in the Update-Latest branch as well as some changes I haven't pushed out to github yet, which also contains some of the code necessary for announcing, so I may not be able to merge your PR without causing merge conflicts between your branch and my branch once I attempt to push those changes.
So funnily enough you kinda ended up doing more work than you had to do.
As a temporary solution though, to get a release out, we could both agree on you publishing the 1.25.1 release (also on BSMG), since we're internally already updating everything to 1.27.0, so there wouldn't be a point for me to push for a 1.25.1 release.

Though with the recent additions of libraries like lapiz and bsml on quest it may be possible that I'll be doing a full rewrite of the mod to get it as close to being like it's PC counterpart as possible.
Which is also in part one of the reasons I haven't updated this since I'm unsure about keeping the current codebase or starting from scratch.
As well as possibly rewriting MpCore and planning on getting MQE to work with Single Player Environments.

@TheShadowEevee
Copy link
Author

I swear I checked for any branches that may have been worked on already; that's what I get for trusting GitHub mobile and not checking again. Since there are plans in the works I'll maintain what I've done on my fork, it's up to you what you think would be best going forward though. I'd be happy to maintain it (looking into bugs / keeping it updated) until a more "official" update is complete depending on what your overall plan is.

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

Successfully merging this pull request may close these issues.

2 participants