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.

Sign up for GitHub

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

Jump to bottom

Friendly fire update #33

Merged
merged 16 commits into from
Nov 18, 2024
Merged

Friendly fire update #33

merged 16 commits into from
Nov 18, 2024

Conversation

Undid-Iridium
Copy link

Description

Describe the changes

x3rt
in ExplodingFlashGrenade.cs ProcessEvent is called
within which you check if (!IndividualFriendlyFire.CheckFriendlyFirePlayer(instance.PreviousOwner, player.ReferenceHub) && !instance.PreviousOwner.SameLife(new(referenceHub)))

The thing is that when the server has Friendly Fire enabled, CheckFriendlyFirePlayerRules returns false if two players are on the same team.
Meaning that Flashbangs do not affect teammates.

TLDR:

Stop using HitboxIdentity.IsEnemy in CheckFriendlyFirePlayerRules when Server.FriendlyFire is true

What is the current behavior? (You can also link to an open issue here)
When the server has Friendly Fire enabled, CheckFriendlyFirePlayerRules returns false if two players are on the same team.

What is the new behavior? (if this is a feature change)
When the server has Friendly Fire enabled, default to NW logic to handle behavior.

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No.

Other information:


Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentations

Submission checklist

  • I have checked the project can be compiled
  • [?] I have tested my changes and it worked as expected

Patches (if there are any changes related to Harmony patches)

  • I have checked no IL patching errors in the console

Other

  • Still requires more testing
    image

I have no intention on a full possible combo test ¯\_(ツ)_/¯

@Undid-Iridium Undid-Iridium changed the base branch from master to dev August 4, 2024 07:31
@IRacle1
Copy link

IRacle1 commented Aug 4, 2024

welcome back😉

@github-actions github-actions bot removed the API label Aug 4, 2024
Misaka-ZeroTwo
Misaka-ZeroTwo reviewed Aug 6, 2024
View reviewed changes
EXILED/Exiled.Events/Patches/Generic/IndividualFriendlyFire.cs Outdated
@@ -89,17 +89,18 @@ public static bool CheckFriendlyFirePlayerRules(Footprint attackerFootprint, Ref

// Return false, no custom friendly fire allowed, default to NW logic for FF. No point in processing if FF is enabled across the board.
if (Server.FriendlyFire)
return HitboxIdentity.IsEnemy(attackerFootprint.Role, victimHub.roleManager.CurrentRole.RoleTypeId);
return false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am confused, if FriendlyFire has been set to true, why return false?

Copy link
Author

Choose a reason for hiding this comment

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

Goes to NW logic

Copy link
Author

Choose a reason for hiding this comment

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

This code's behavior is, if true, we are doing custom FF logic, do not use NW logic. If false, we're not using custom ff logic, or we got to the end and we didn't find any rules we could follow, for safety, return false and use NW logic.

Copy link
Author

Choose a reason for hiding this comment

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

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Goes to NW logic

Well iirc when FriendlyFire is false, the client doesn't even send the damage message to the server when both players are in the same team. So it wouldn't really work?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that HitboxIdentity.IsDamageable is not used in the Flashbang event, the ProcessEvent method mentioned above is used,

What's the reason why it's not used

In that Flashbang patch, the codes for adding targets in base game are skipped

I can see that, what I meant was, why

legacy code moment

Copy link
Author

Choose a reason for hiding this comment

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

Option 2 it is?

Copy link

@louis1706 louis1706 Aug 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Option 2 it is?

if you put option 2 don't use Footprint:Hub but Footprint:Role that very important for things like grenade when you get your rôle changed (i don't really get the difference between all the option but that very important to not use ReferenceHub here)

Copy link
Author

Choose a reason for hiding this comment

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

Done

Copy link

Choose a reason for hiding this comment

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

Done

thanks and even if you choose an other option still does this change :3 because it's needed

@Misaka-ZeroTwo Misaka-ZeroTwo mentioned this pull request Aug 6, 2024
Closed
8 tasks
louis1706
louis1706 reviewed Aug 7, 2024
View reviewed changes
Copy link

@louis1706 louis1706 left a comment

Choose a reason for hiding this comment

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

That 100% sure

EXILED/Exiled.Events/Patches/Generic/IndividualFriendlyFire.cs Outdated Show resolved Hide resolved
louis1706
louis1706 approved these changes Oct 19, 2024
View reviewed changes
Copy link

@louis1706 louis1706 left a comment

Choose a reason for hiding this comment

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

I am scared if this contains bug

@Undid-Iridium
Copy link
Author

I am scared if this contains bug

Don't merge then - wait for a tester. I'm in no rush to have this merged or I'll close it and you can use x3rt solution

@louis1706
Copy link

I am scared if this contains bug

Don't merge then - wait for a tester. I'm in no rush to have this merged or I'll close it and you can use x3rt solution

i will wait for testing

@adarkaz
Copy link

adarkaz commented Oct 27, 2024

i tested, works with NPC

@github-actions github-actions bot removed the Events label Nov 18, 2024
@louis1706 louis1706 merged commit ef52b45 into ExMod-Team:dev Nov 18, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Misaka-ZeroTwo Misaka-ZeroTwo Misaka-ZeroTwo left review comments

@Misfiy Misfiy Misfiy approved these changes

@louis1706 louis1706 louis1706 approved these changes

Assignees

@Undid-Iridium Undid-Iridium

Labels
Transpiler
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Undid-Iridium @IRacle1 @louis1706 @adarkaz @Misaka-ZeroTwo @Misfiy