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

New Shot patch #289

Merged
merged 2 commits into from
Dec 3, 2024
Merged

Conversation

Banalny-Banan
Copy link

@Banalny-Banan Banalny-Banan commented Dec 1, 2024

Description

Describe the changes
Added new Shot patch

What is the current behavior? (You can also link to an open issue here)
Current Shot patch is using Physics.Raycast in its constructor and is missing crucial CanHurt field that is widely used in plugins

What is the new behavior? (if this is a feature change)
Mentioned issues are fixed

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Yes, it removes MaxDistance field from ShotEventArgs. This field is just as relevant as including raycast mask in the event (not at all)


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

@BoltonDev
Copy link

MaxDistance isn't a constant value, the value is different if the firearm used is a disruptor

@Banalny-Banan
Copy link
Author

Banalny-Banan commented Dec 1, 2024

MaxDistance isn't a constant value, the value is different if the firearm used is a disruptor

I know, but this information is still not a part of the Shot. It's a parameter of the firearm just like fire rate or max ammo. If plugins want to access the parameters of the Firearm they can do so directly.

@BoltonDev
Copy link

Also i don't think CanHurt should be a part of the ShotEvent as we can already do that in the HurtingEvent by checking the DamageHandler. ShotEvent represents the action of firing a firearm

@Banalny-Banan
Copy link
Author

CanHurt is useful when you are making a weapon with a custom way of dealing damage and want to cancel the default. It is possible in Hurting, but why not have it here if we can? Also its backward compatibility since this field was there for more than a year.

@Banalny-Banan Banalny-Banan marked this pull request as ready for review December 2, 2024 16:46
@Banalny-Banan
Copy link
Author

@BoltonDev Ready for review

@BoltonDev
Copy link

BoltonDev commented Dec 2, 2024

It looks good, just could confirm that you have tested your changes and everything is working as expected?
image

@Banalny-Banan
Copy link
Author

@BoltonDev Yes, its tested and working correctly

@BoltonDev BoltonDev merged commit cb67978 into ExMod-Team:scpsl14 Dec 3, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants