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

Add export to allow external scripts to get killer data #97

Merged
merged 6 commits into from
Jul 25, 2024

Conversation

JHansen2000
Copy link
Contributor

Description

Add export to allow external scripts to get killer data

Checklist

  • I have personally loaded this code into an updated Qbox project and checked all of its functionality.
  • My pull request fits the contribution guidelines & code conventions.

@JHansen2000
Copy link
Contributor Author

Sorry for the messy commits, I had some old code I was playing around with that was fixed by someone else's changes and I forgot to remove that old code prior to opening this PR.

Copy link
Contributor

@Manason Manason left a comment

Choose a reason for hiding this comment

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

Would this be better as an event instead?

@mafewtm
Copy link
Member

mafewtm commented Jul 22, 2024

What is the usecase for this as well?

@JHansen2000
Copy link
Contributor Author

This is helpful for developers to be able to retrieve the killer's data for other scripts (i.e. a deathscreen that displays killer name and how they killed you). Without this export, external scripts must listen to the native CEventNetworkEntityDamage and parse it on their own, which is very inefficient since Qbox is doing this already.

I think this is better as an export, rather than an event, but am definitely open to discussion if you think an event would be better. What is the benefit of using an event over an export for this?

@Manason
Copy link
Contributor

Manason commented Jul 22, 2024

This is helpful for developers to be able to retrieve the killer's data for other scripts (i.e. a deathscreen that displays killer name and how they killed you). Without this export, external scripts must listen to the native CEventNetworkEntityDamage and parse it on their own, which is very inefficient since Qbox is doing this already.

I think this is better as an export, rather than an event, but am definitely open to discussion if you think an event would be better. What is the benefit of using an event over an export for this?

How would this be used? The event makes sense to me so that other scripts can listen for an onDeath event if they care to take action when the player dies. Otherwise, they will have to call the export in a loop to continuously check.

@JHansen2000
Copy link
Contributor Author

This is helpful for developers to be able to retrieve the killer's data for other scripts (i.e. a deathscreen that displays killer name and how they killed you). Without this export, external scripts must listen to the native CEventNetworkEntityDamage and parse it on their own, which is very inefficient since Qbox is doing this already.

I think this is better as an export, rather than an event, but am definitely open to discussion if you think an event would be better. What is the benefit of using an event over an export for this?

How would this be used? The event makes sense to me so that other scripts can listen for an onDeath event if they care to take action when the player dies. Otherwise, they will have to call the export in a loop to continuously check.

I see where you're coming from. My scripts listen for changes to the client state bag instead of an event, so I only call the export when the state changes. I think this is good practice, but let me know if you disagree.

@Manason
Copy link
Contributor

Manason commented Jul 22, 2024

This is helpful for developers to be able to retrieve the killer's data for other scripts (i.e. a deathscreen that displays killer name and how they killed you). Without this export, external scripts must listen to the native CEventNetworkEntityDamage and parse it on their own, which is very inefficient since Qbox is doing this already.

I think this is better as an export, rather than an event, but am definitely open to discussion if you think an event would be better. What is the benefit of using an event over an export for this?

How would this be used? The event makes sense to me so that other scripts can listen for an onDeath event if they care to take action when the player dies. Otherwise, they will have to call the export in a loop to continuously check.

I see where you're coming from. My scripts listen for changes to the client state bag instead of an event, so I only call the export when the state changes. I think this is good practice, but let me know if you disagree.

Wouldn't it be more streamlined to have a named event with additional data (such as the killer data) rather than state bag listener + export?

@JHansen2000
Copy link
Contributor Author

It would be more streamlined. I will update this tonight

@JHansen2000
Copy link
Contributor Author

Sorry for the delay, this has been added simply as an event trigger. I do think this (and other events/exports) should be added to documentation somewhere but did not want to modify the README without approval.

@JHansen2000 JHansen2000 requested a review from Manason July 24, 2024 01:46
@JHansen2000
Copy link
Contributor Author

I passed the data as props to both OnDeath and StartLastStand and from there into the associated client & server events. It's all optional but is definitely very helpful. Hope that looks okay and this wasn't too much of a headache for you!

@Manason Manason merged commit c0eed4c into Qbox-project:main Jul 25, 2024
2 checks passed
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.

3 participants