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

Support multiple spaces in custom damage source ID display names #835

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

Dinorush
Copy link
Contributor

Simple change that allows the custom damage source IDs to support multiple spaces rather than only one. The logic relies on string commands to sync them, so having spaces in the names causes issues. Currently it only replaces one space.

For instance, RegisterWeaponDamageSource( "mp_titanweapon_brute4_quad_rocket", "Brute4 Quad Rocket" ) results in a client crash:

damage_type_crash_example

With the PR, it works:

damage_type_example

@GeckoEidechse
Copy link
Member

Uh, is there a quick way to test this PR without installing a custom mod and modifying it? ^^"

@Dinorush
Copy link
Contributor Author

Base Northstar uses this function to register Peacekraber and Kunai (under Northstar.Custom -> vscripts -> sh_northstar_custom_precache). So you could modify the Peacekraber to use a display name with two spaces to verify that it displays properly in killfeed.

To test if the crash is fixed, all you need to do is have a custom damage ID with 2 spaces in the display name and start the match. The weapon using it need not exist. It'll crash on map start if unfixed since it happens when the client connects.

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

Thanks for the instructions. Confirmed working in testing now.
I checked that using a custom damage source ID display name with more than one game crashes the game and that this PR fixes it.

I did NOT verify that it displays properly in killfeed as my development setup rn doesn't allow me to run the game multiple times and setting up a playtest for this was too cumbersome. So I just hope that part is gonna be fine lol

@GeckoEidechse GeckoEidechse added needs code review Changes from PR still need to be reviewed in code almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge READY TO MERGE This mergeable right now and removed almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge labels Aug 14, 2024
Copy link
Member

@F1F7Y F1F7Y left a comment

Choose a reason for hiding this comment

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

The changes look fine

@GeckoEidechse GeckoEidechse removed needs code review Changes from PR still need to be reviewed in code READY TO MERGE This mergeable right now labels Aug 14, 2024
@GeckoEidechse GeckoEidechse merged commit 7aa3958 into R2Northstar:main Aug 14, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants