-
Notifications
You must be signed in to change notification settings - Fork 110
Add role checks (like IsScp) to Role Extensions / RoleTypeId #177
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
Changes from 14 commits
974bd6d
80b4094
619fa0c
a870390
8c9ad97
2d85206
e3cd71e
dd71f42
44feae9
f67746c
735d01b
71f2498
c03ccc1
bcdfb3b
144cf9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,6 +123,48 @@ public static bool TryGetRoleBase<T>(this RoleTypeId roleType, out T roleBase) | |
| /// <returns>Returns whether <paramref name="roleType"/> is an <see cref="IFpcRole"/> or not.</returns> | ||
| public static bool IsFpcRole(this RoleTypeId roleType) => roleType.GetRoleBase() is IFpcRole; | ||
|
|
||
| /// <summary> | ||
| /// Checks if the role is an SCP role. | ||
| /// </summary> | ||
| /// <param name="roleType">The <see cref="RoleTypeId"/>.</param> | ||
| /// <returns>A boolean which is true when the role is an SCP role.</returns> | ||
| public static bool IsScp(this RoleTypeId roleType) => roleType.GetTeam() == Team.SCPs; | ||
|
|
||
| /// <summary> | ||
| /// Checks if the role is a dead role. | ||
| /// </summary> | ||
| /// <param name="roleType">The <see cref="RoleTypeId"/>.</param> | ||
| /// <returns>A boolean which is true when the role is a dead role.</returns> | ||
| public static bool IsDead(this RoleTypeId roleType) => roleType.GetTeam() == Team.Dead; | ||
|
|
||
| /// <summary> | ||
| /// Checks if the role is an NTF role. | ||
| /// </summary> | ||
| /// <param name="roleType">The <see cref="RoleTypeId"/>.</param> | ||
| /// <returns>A boolean which is true when the role is an NTF role. Does not include Facility Guards.</returns> | ||
| public static bool IsNtf(this RoleTypeId roleType) => roleType.GetTeam() == Team.FoundationForces && roleType != RoleTypeId.FacilityGuard; | ||
|
|
||
| /// <summary> | ||
| /// Checks if the role is a Chaos role. | ||
| /// </summary> | ||
| /// <param name="roleType">The <see cref="RoleTypeId"/>.</param> | ||
| /// <returns>A boolean which is true when the role is a Chaos role.</returns> | ||
| public static bool IsChaos(this RoleTypeId roleType) => roleType.GetTeam() == Team.ChaosInsurgency; | ||
|
|
||
| /// <summary> | ||
| /// Checks if the role is a military role (Chaos Insurgency or NTF). | ||
| /// </summary> | ||
| /// <param name="roleType">The <see cref="RoleTypeId"/>.</param> | ||
| /// <returns>A boolean which is true when the role is a military role. Does not include Facility Guards.</returns> | ||
| public static bool IsMilitary(this RoleTypeId roleType) => roleType.IsNtf() || roleType.IsChaos(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just does I know this goes against what miki said but that what basegame believe for armor consumption of stamina for example There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Idk can we count tutorial as civilian (iirc when I checked it last time
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tutorial is not civilian, no. so it will count as military, this change shouldn't be done @louis1706 as what I've done is perfect, this is just adding more work and more calculations for no reason. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. technically guard is a military class, I've said otherwise before because it doesn't make sense to me but that's how they're classified on the wiki, this method should reflect that
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Way to late to say that now @Mikihero 😭 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to make it work you can just add
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, but this PR has been merged, if I could directly commit I would |
||
|
|
||
| /// <summary> | ||
| /// Checks if the role is a civilian role (Scientists and Class-D). | ||
| /// </summary> | ||
| /// <param name="roleType">The <see cref="RoleTypeId"/>.</param> | ||
| /// <returns>A boolean which is true when the role is a civilian role.</returns> | ||
| public static bool IsCivilian(this RoleTypeId roleType) => roleType == RoleTypeId.ClassD || roleType == RoleTypeId.Scientist; | ||
|
|
||
| /// <summary> | ||
| /// Gets a random spawn point of a <see cref="RoleTypeId"/>. | ||
| /// </summary> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently this includes Guards because of the other thing I mentioned, I think you should specify that this doesn't (or does if it's intended) include them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should grab guards as they are military, but if you want the docs to be changed to mention guards are included in both foundation and military I can do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally I wouldn't count guards as military, that's why I'd like you to specifically mention if they're counted or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'll add smth to remove guards from military as I agree it's weird for them to be in there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing from IsNtf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you could specify in its docs that doesn't include guards that'd be great as well since some people might be confused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done