Skip to content

Some improvements#53

Merged
Galster-dev merged 8 commits into
CrowdedMods:masterfrom
AlchlcDvl:master
Oct 23, 2023
Merged

Some improvements#53
Galster-dev merged 8 commits into
CrowdedMods:masterfrom
AlchlcDvl:master

Conversation

@AlchlcDvl

@AlchlcDvl AlchlcDvl commented Jul 13, 2023

Copy link
Copy Markdown
Contributor

This pr contains

  • Adding page numbers to the Shapeshifter and Vitals menus like it's there for Meetings
  • Added a mono behaviour for Vitals (and removing its related Postfix in the process)
  • The minimum lobby size you can set is 5 with this mod instead of the vanilla 4 so it fixes that (by reducing it to 3 to make it funny)
  • Pages now loop instead of clamping them to either end
  • Adds the Dependency flags from Reactor (from the suggestions channel in the crowded discord)
  • Fixed (needs testing) the occasional page counter spamming in the meeting's timer text

Haven't been able to test it fully since 15+ player lobbies are rare, but through some solo testing using bots they work for the most part

@Galster-dev Galster-dev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for you contribution. Due to stability and compatibility level we're aiming at, we cannot accept this PR without noted changes. I'm absolutely down to implement those changes myself, but if you want to do it, I'm not complaining.

Comment thread src/CrowdedMod/Components/AbstractPagingBehaviour.cs Outdated
Comment thread src/CrowdedMod/Components/AbstractPagingBehaviour.cs Outdated
Comment thread src/CrowdedMod/Components/ShapeShifterPagingBehaviour.cs Outdated
Comment thread src/CrowdedMod/Components/ShapeShifterPagingBehaviour.cs Outdated
Comment thread src/CrowdedMod/Components/VitalsPagingBehaviour.cs Outdated
Comment thread src/CrowdedMod/Components/VitalsPagingBehaviour.cs Outdated
Comment thread src/CrowdedMod/CrowdedMod.csproj Outdated
Comment thread src/CrowdedMod/CrowdedMod.csproj Outdated

var tmp = playerButton.GetComponentInChildren<TextMeshPro>();
var newValue = Mathf.Max(byte.Parse(tmp.text) - 10, byte.Parse(playerButton.name));
var newValue = Mathf.Max(byte.Parse(tmp.text) - 10, byte.Parse(playerButton.name) - 2);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd stick to 4 tbh, but 3 is fine I guess

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

honestly i thought of it as funny seeing a 3 player lobby, it's merely a joke-ish change which i'm willing to change to 4 if needed

Comment thread src/CrowdedMod/Patches/PagingPatches.cs Outdated
@AlchlcDvl AlchlcDvl requested a review from Galster-dev July 13, 2023 22:15
@AlchlcDvl

Copy link
Copy Markdown
Contributor Author

oops accidentally requested review my bad will make those changes

@AlchlcDvl

Copy link
Copy Markdown
Contributor Author

im thinking of moving

PageText.text = $"({PageIndex + 1}/{MaxPageIndex + 1})";

into the OnPageChanged() method instead of Update()

@AlchlcDvl

AlchlcDvl commented Jul 20, 2023

Copy link
Copy Markdown
Contributor Author

worked on it a wee bit more, and went ahead with my earlier idea

@AlchlcDvl

Copy link
Copy Markdown
Contributor Author

so is this pr good enough to be merged? or is there something else that's to be done

@AlchlcDvl

Copy link
Copy Markdown
Contributor Author

@Galster-dev made your requested changes

{
__instance.CrewArea.SetCrewSize(opts.MaxPlayers, opts.NumImpostors);
}
__instance.CrewArea?.SetCrewSize(opts.MaxPlayers, opts.NumImpostors);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

? operator bypasses Unity lifetime checks

@Galster-dev Galster-dev merged commit 6d5ef61 into CrowdedMods:master Oct 23, 2023
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