Skip to content

Beat Saber 1.11.1 Update (and other things)#19

Open
Zingabopp wants to merge 26 commits into71:masterfrom
Zingabopp:bs-1.11.1
Open

Beat Saber 1.11.1 Update (and other things)#19
Zingabopp wants to merge 26 commits into71:masterfrom
Zingabopp:bs-1.11.1

Conversation

@Zingabopp
Copy link
Copy Markdown

@Zingabopp Zingabopp commented Sep 13, 2020

  • Converted to BSIPA and updated for Beat Saber 1.11.1.

    • Depends on SongCore and BS_Utils.
  • Includes fixes and settings from this fork.

  • Improvements:

    • In-game settings UI.
    • Song-specific settings in tab on GameplayModifiers panel.
      • Set timeOffset and timeScale
      • Button to fetch lyrics from online services.
      • Button to save song lyrics settings.
      • Button to test lyric settings from the menu.
    • Matches song timescale (faster/slower song).
    • Option to save fetched lyrics to JSON in the song directory.
      • lyrics.json can be either a JSON array as before, or a JSON object containing the array and timeOffset and timeScale settings.
    • Added shake, color, size, and position settings for lyric text.

Let me know if you want me to do a version bump.
Closes #17
BeatSinger-1.2.0-bs1.11.1-001c3b2.zip

@Zingabopp
Copy link
Copy Markdown
Author

Ok, I think I'm finally satisfied with it. 🙄

@71
Copy link
Copy Markdown
Owner

71 commented Sep 14, 2020

Thanks for all your work! I took a quick look but there are a lot of changes so I'll need some time to review everything. I'm also unable to test changes since I don't have a VR headset anyway, so I'd be glad if someone else can confirm that the changes work as intended.

As for versioning, I originally wanted to use the version scheme <Beat Saber version>.<Beat Singer patch>, so the new version should be 1.11.1.0?

@Zingabopp
Copy link
Copy Markdown
Author

You can test with mouse/keyboard if you still own the game using FPFCToggle and MusicEscape.
For the release version, BeatMods requires SemVer for uploaded mods and iirc BSIPA needs it if a mod had BeatSinger as a dependency (unlikely). If neither of those matter then 1.11.1.0 is fine.

@ChipWolf
Copy link
Copy Markdown

ChipWolf commented Sep 14, 2020

I've been testing this, there's a strange issue with the BeatSinger mod menu for the song doesn't close correctly if you switch view. It seems to remain open behind the other menus.

Other than that, works perfectly. Thanks for the update!
It would be cool if there was a settings toggle to disable the BeatSinger toggle button or change it to a chord (I keep accidentally hitting it mid-song on the knuckles).

Many thanks

@Zingabopp
Copy link
Copy Markdown
Author

I'll probably do something else with the song testing thing. Too much bug potential with moving the main screen out of view.

@ChipWolf
Copy link
Copy Markdown

On the Index, it turns out the toggle is bound to squeezing the left controller or pressing the A button on the left controller, squeezing the controller is probably what I kept repeatedly triggering accidentally.

@Zingabopp
Copy link
Copy Markdown
Author

Latest link up there should fix the lyric testing UI problems.

}
}

public static JSONArray ToJsonArray(this IEnumerable<Subtitle> subtitles)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this much, since seeing a ToJsonArray in the wild make it seem as if ToJsonArray is a generic function that works for all IEnumerables (and that looks like it "pollutes" the namespace).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it makes a difference, the extension only works (and shows up in Intellisense) for IEnumerable<Subtitle>. My thinking was Subtitle doesn't care how it's being serialized, so I made that method an extension instead of including it in the Subtitle class.

Copy link
Copy Markdown
Owner

@71 71 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zingabopp Thanks! I have a few requests but I only had the time to review the first three commits.

Edit: I forgot to mention this before but I think you should set the new version to 1.2.0, since I was already using 1.1.0.0. Also, this is a pretty significant change so if you're comfortable with it, please add your name to the "authors" sections in metadata.

@ChipWolf There is already a setting for changing the toggle key, but I forgot to document it. In the mod preferences, you can set ToggleKeyCode = 1, replacing 1 with a value from https://github.com/71/BeatSinger/blob/master/BeatSinger/Helpers/ConInput.cs (the values appear to be available here).

Comment thread BeatSinger/LyricsComponent.cs Outdated

var sceneSetupData = (GameplayCoreSceneSetupData)SceneSetupDataField.GetValue(sceneSetup);
var sceneSetupData = Access_SceneSetupData(ref sceneSetup);
//(GameplayCoreSceneSetupData)SceneSetupDataField.GetValue(sceneSetup);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If your new code works, I think you can remove this (same thing below).

Comment thread BeatSinger/Settings.cs

if (inputDevices.Exists(x => x.name.IndexOf("rift", StringComparison.InvariantCultureIgnoreCase) != -1))

if (XRDevice.model.IndexOf("rift", StringComparison.InvariantCultureIgnoreCase) != -1)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think XRDevice.model is now deprecated.

@@ -0,0 +1,65 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this file come from? Same question for Directory.Build.props. I suppose it's some shared template?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's a set of build targets I'm including in the BeatSaberModdingTools templates. It's nicer than using post build events when other people are working on the project because it uses BeatSaberDir for the output and does some other things like verifying the version in manifest.json matches AssemblyVersion and automatically zips releases for BeatMods

/// <summary>
/// Container for subtitles.
/// </summary>
public sealed class SubtitleContainer : IEnumerable<Subtitle>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at its usage, I think we should prefer a completely immutable data structure, so either we only build it when we have all necessary information, or we add a WithSubtitles function which returns a new Container. Overall I like this idea though!

Comment thread BeatSinger/LyricsComponent.cs Outdated
if (Settings.VerboseLogging)
Plugin.log?.Debug($"{level.songName} is {(customLevel != null ? "" : "not ")}a custom level.");
if (customLevel != null && LyricsFetcher.GetLocalLyrics(customLevel.customLevelPath, subtitles))
SubtitleContainer container = null;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we'd only have the SubtitleContainer or the subtitles to avoid having to think about both variables in all those possible branches.

The ideal solution IMO is a SubtitleContainer.Builder class that we'd use everywhere instead of List<Subtitle>, but this may be a lot of work for not much.

Comment thread BeatSinger/Helpers/Extensions.cs Outdated
}
return array;
}
public static JSONObject ToJson(this SubtitleContainer subtitles)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and in some other places: please separate methods with an empty line.

(Also in other places but the GitHub code viewer doesn't make it easy to see them: please make sure all indentation is made with spaces instead of tabs, and please make sure all files end with a newline)

@MaverickMartyn
Copy link
Copy Markdown

Hi there guys. I miss this mod, and I was wondering what the holdup was?
Is there anything I can do to help?

@71
Copy link
Copy Markdown
Owner

71 commented Jan 14, 2021

I haven't had time to test the changes, especially since I no longer have a VR headset (there are ways to test it, but I haven't gotten around to it). I think the best thing you (probably @Zingabopp if they're interested, since they already fixed what was needed) can do is fork the project. I'll mention the fork on the README if you decide to do that.

@MaverickMartyn
Copy link
Copy Markdown

In that case I'll wait a bit and see if @Zingabopp gets back to us. I wouldn't want to steal their thunder by grabbing their changes before they have a chance. :)
Is the project ready to build as is? That way I could compile it for myself until a release is available.

@Mint-Martini
Copy link
Copy Markdown

Any word on this, Singing along it beat saber really is one of my favorite things haha

@71
Copy link
Copy Markdown
Owner

71 commented Feb 14, 2021

Given that @Zingabopp hasn't replied in over a month, I think you can take it @MaverickMartyn :)

@Neo4114, I took a very quick look and the mod that @Zingabopp linked in the first comment doesn't seem to do anything sketchy. I don't know if it works, but you can try it out.

@Mint-Martini
Copy link
Copy Markdown

Mint-Martini commented Feb 14, 2021

@71 Thanks so much for the reply!,
I just dropped the file in my Plugins folder but it doesn't seem to work
I tried compiling it on my end and i am having some issues to build ( Might be me though haha ) Hopefully someone more experienced can pick it up!

to be fair i am running this on the newest version of Beat saber aswell

@71
Copy link
Copy Markdown
Owner

71 commented Feb 14, 2021

Did you drop the .zip file or the .dll inside of it? Make sure it's the .dll. And yeah if the Beat Saber version has changed since then, it might no longer be compatible :(

@Mint-Martini
Copy link
Copy Markdown

Sadly yes, :( i just followed standard procedure when dealing with mods ,
Beat Saber is on 1.13.2 (I think ) Ill see if i can compile it, i am pretty familiar with code just not C# , or objects related to beat saber (Or unity in general )

@71
Copy link
Copy Markdown
Owner

71 commented Feb 14, 2021

Alright, good luck! Start by changing BeatSaberDir in BeatSinger/BeatSinger.csproj, it should help resolve most of the dependencies automatically. After that you can try building the project with dotnet and try to fix whatever issues may arise.

@Mint-Martini
Copy link
Copy Markdown

Just wanted to give you an update ,
After resolving the dependencies here's what I got ,

It seems that these two dependencies are no longer supported in the base game, Could you give any word on what these actually do (or were intended to do)

My plan would be as follows ,

  1. Figure out what the previous entities are
  2. Find the equivalent class or class rewrite
  3. ????
  4. Profit haha

Thanks again for explaining some things

Not sure on why Input is being a pain , ill look into that myself
image

Here they are in code inside Lyrics component
image

@71
Copy link
Copy Markdown
Owner

71 commented Feb 15, 2021

Yep, looks like the API slightly changed... To find the new equivalent entities I'd recommend using dnSpy and looking into the assemblies of Beat Saber. That's how I did it:

  1. Determine what i was looking for (e.g. the menu or the Flying Text effect).
  2. Find it in the assembly with dnSpy.
  3. Find classes that contain a field of what I'm looking for. If that class isn't available, start again with that class until I find a class that I can use with FindObjectOfType.

Not sure about Input either. I don't remember, but maybe it's a deprecated Unity API?

@71 71 mentioned this pull request Sep 22, 2021
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.

Not currently working with BeatSaber 1.11.1

5 participants