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

Generate Impostor Modifiers First #171

Open
wants to merge 1 commit into
base: mainline
Choose a base branch
from

Conversation

VincentVision
Copy link

You generate your modifer impostor after the globals, so that if a global goes on an impostor, it will not be able to have the modifier impostor if you have 2 for 2 impostors it will create a conflict example, I put the 2 modifiers impostor at 100% and I have 2 impostors in the game, one of them is designated button barry, only one of the 2 imposters modifiers will be present while I expect to have both.

You generate your modifer impostor after the globals, so that if a global goes on an impostor, it will not be able to have the modifier impostor if you have 2 for 2 impostors it will create a conflict example, I put the 2 modifiers impostor at 100% and I have 2 impostors in the game, one of them is designated button barry, only one of the 2 imposters modifiers will be present while I expect to have both.
@@ -892,6 +897,15 @@ public static void Prefix([HarmonyArgument(0)] ref Il2CppReferenceArray<GameData

if (Check(CustomGameOptions.GrenadierOn))
ImpostorRoles.Add((typeof(Grenadier), CustomRPC.SetGrenadier, CustomGameOptions.GrenadierOn));
#endregion
#region Impostor Modifiers //just for better organization
if (Check(CustomGameOptions.AnthropomancerOn))
Copy link
Owner

Choose a reason for hiding this comment

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

Anthropomancer isn't an impostor-only modifier

@@ -172,19 +172,24 @@ private static void GenEachRole(List<GameData.PlayerInfo> infected)
Role.Gen<Role>(typeof(Crewmate), executioner, CustomRPC.SetExecutioner);
}

var canHaveModifier = PlayerControl.AllPlayerControls.ToArray().ToList();
List<PlayerControl> canHaveModifier = PlayerControl.AllPlayerControls.ToArray().ToList().FindAll(player => player.Is(Faction.Impostors));
Copy link
Owner

Choose a reason for hiding this comment

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

This removing and adding from the canHaveModifier list feels very brittle and easy to mess up in the future. You've found a real bug, but I don't like this fix. I think we need to find a better method of bookkeeping.

Partially this is because the existing code is a bit messy. Probably the right answer is to do the crewmates and the impostors separately, and combine those lists. I suspect part of the key is to create separate lists and combine rather than have a single list and add to it. I dislike modifying the list in this way for the same reason that I like final variables; you can look at a line of code and easily run backward through the code in your head and figure out what happened to get there.

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.

None yet

2 participants