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

Shrapnel smooth movement [WIP] #6821

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

Doubleumc
Copy link
Contributor

@Doubleumc Doubleumc commented Jul 30, 2024

About the pull request

Shrapnel movement is now smoothed by an animate().

This is a follow on from #4986 , attempting to resolve the performance issue that forced shrapnel to be excluded: #4986 (comment)

Replaced the visual-effect-only shrapnel /datum/ammo/bullet/shrapnel/light/effect with a particle system that imitates their look. The two main differences are the particles don't collide with anything and all are using the "sparks" icon instead of the "tracer" icon because the tracer has a clear direction to it and particles can't easily coordinate angle and facing.

tracer on left, sparks on right
image

TM yes, full merge no. The current particles implementation is single-purpose for testing. Assuming all works well and its not causing client performance issues, will bring in a more robust solution such as in TGMC.

Surely using a purely visual effect is less taxing than using full-up objects. Surely BYOND is sensible in that regard.

Right?

Explain why it's good for the game

Smoother apparent movement for shrapnel gives a more appealing look.

Testing Photographs and Procedure

Screenshots & Videos

Server memory stats and average client render.

baseline:

Server mem usage:
prototypes:
	obj: 3.75 MB (19,601)
	mob: 2.08 KB (133)
	proc: 15.3 MB (31,429)
	str: 9.86 MB (163,347)
	appearance: 180 KB (4,807)
	filter: 16.4 KB (7)
	id array: 38.6 MB (76,961)
	map: 6.37 MB (100,150,5)
objects:
	mobs: 5.59 KB (5)
	objs: 2.7 MB (8,520)
	datums: 3.39 MB (29,822)
	images: 2.62 MB (11,562)
	lists: 13 MB (86,256)
	procs: 7.18 KB (23)

image

after cluster OB, current master:

Server mem usage:
prototypes:
	obj: 3.75 MB (19,601)
	mob: 2.08 KB (133)
	proc: 15.3 MB (31,429)
	str: 9.88 MB (163,347)
	appearance: 295 KB (9,027)
	filter: 16.4 KB (7)
	id array: 38.6 MB (81,772)
	map: 6.53 MB (100,150,5)
objects:
	mobs: 5.59 KB (5)
	objs: 7.75 MB (17,446)
	datums: 4.08 MB (32,403)
	images: 2.63 MB (11,601)
	lists: 13.5 MB (95,057)
	procs: 6.89 KB (22)

image

after cluster OB, initial commit:

Server mem usage:
prototypes:
	obj: 3.75 MB (19,601)
	mob: 2.08 KB (133)
	proc: 15.3 MB (31,429)
	str: 9.88 MB (163,348)
	appearance: 1.34 MB (43,820)
	filter: 16.4 KB (7)
	id array: 38.6 MB (76,960)
	map: 6.53 MB (100,150,5)
objects:
	mobs: 5.59 KB (5)
	objs: 7.95 MB (17,524)
	datums: 3.54 MB (30,401)
	images: 2.63 MB (11,599)
	lists: 13.5 MB (95,167)
	procs: 6.89 KB (22)

image

after cluster OB, particles experiment commit:

Server mem usage:
prototypes:
	obj: 3.75 MB (19,589)
	mob: 2.08 KB (133)
	proc: 15.2 MB (31,190)
	str: 9.87 MB (163,299)
	appearance: 209 KB (6,457)
	filter: 16.4 KB (7)
	id array: 38.3 MB (76,816)
	map: 6.53 MB (100,150,5)
objects:
	mobs: 5.59 KB (5)
	objs: 5.88 MB (15,559)
	datums: 3.56 MB (30,860)
	images: 2.62 MB (11,571)
	lists: 13.3 MB (92,035)
	procs: 6.89 KB (22)

image

On master gains about 120KB in appearances.
On initial commit gains about 1.2MB in appearances. Unsure why. Maybe the mutable_appearances count and aren't being discarded?
On particles experiment commit gains about 40KB in appearances. Presumably the reduction is due to the client handling generating some of those appearances instead.

No significant difference on average render times for baseline, master, or initial commit.
Particles cause a moderate render time increase for particles experiment commit during a cluster OB.

Works without issue on my local machine. Unable to test at appropriate scale. One TM and one cluster OB should give a definitive answer.

Changelog

🆑
code: shrapnel smoothly animates their movement
code: visual-effect-only shrapnel replaced by clientside particles
/:cl:

@github-actions github-actions bot added the Code Improvement Make the code longer label Jul 30, 2024
Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

fps for me seems approximately the same. Self cpu of animate_flight does increase by .24 (or 0.0000064205232035814 per call) but shrug

@Drulikar Drulikar added the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Aug 1, 2024
@Doubleumc
Copy link
Contributor Author

I don't think local testing can determine anything beyond "this does/doesn't horrifically break something". The issue with shrapnel only showed up during a cluster OB when there were 100+ people on the server. The best I can figure is it had something to do with the amount of appearance data being sent out so that's what I focused on reducing. Don't have any tools that can test that, though.

cm13-github added a commit that referenced this pull request Aug 1, 2024
cm13-github added a commit that referenced this pull request Aug 1, 2024
cm13-github added a commit that referenced this pull request Aug 2, 2024
cm13-github added a commit that referenced this pull request Aug 2, 2024
@Doubleumc
Copy link
Contributor Author

Looks like the same outcome as the last time it was tried: cluster OBs cause the projectile system to chug.
image
image
Unfortunate, but was worth a shot.

@Drulikar
Copy link
Contributor

Drulikar commented Aug 3, 2024

Feel free to close if you aren't happy with the result or don't see any way to salvage it.

@Drulikar Drulikar removed the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Aug 3, 2024
@Drulikar Drulikar marked this pull request as draft August 3, 2024 01:35
didn't have any beneficial effect
surely a purely visual effect is less intensive than using objects, right?
@Doubleumc Doubleumc changed the title Shrapnel smooth movement Shrapnel smooth movement [WIP] Aug 4, 2024
@Doubleumc Doubleumc marked this pull request as ready for review August 4, 2024 06:37
Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

This drastically reduces the damage delt by a cluster and just visually doesn't really have much happening at all.

@Drulikar Drulikar marked this pull request as draft August 4, 2024 08:57
@Doubleumc
Copy link
Contributor Author

Apparently I didn't actually submit my initial comment update, fixed now.

This drastically reduces the damage delt by a cluster and just visually doesn't really have much happening at all.

I'm not sure what you mean by drastically reducing damage. The only shrapnel being replaced is this and its subtypes:

/datum/ammo/bullet/shrapnel/light/effect/ // no damage, but looks bright and neat
name = "sparks"
damage = 1 // Tickle tickle

Each cluster OB explosion is 350 damage. It can generate up to 18 shrapnel of this kind, for at most 18 damage if it all hits something and isn't immediately absorbed by armor.

I'm not sure what you mean by not much happening visually. The exact same amount of shrapnel is generated, spreading out in approximately the same way, travelling approximately the same distance. The only thing you'd miss is a shrapnel hitting something and it popping up a hit marker.
Cluster OB on master:
image
Cluster OB on particles experiment:
image

@Drulikar Drulikar added Testmerge Candidate we'll test this while you're asleep and the server has 10 players Needs Testing Need to test it on the guinea pigs (production server) labels Aug 5, 2024
Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

Code looks fine, and seems to test and look fine. But will be on TM for people to get a feel for whether its too costly for client fps over the old.

@Doubleumc Doubleumc marked this pull request as ready for review August 5, 2024 06:24
cm13-github added a commit that referenced this pull request Aug 5, 2024
cm13-github added a commit that referenced this pull request Aug 5, 2024
cm13-github added a commit that referenced this pull request Aug 7, 2024
cm13-github added a commit that referenced this pull request Aug 7, 2024
@Drulikar Drulikar added this pull request to the merge queue Aug 7, 2024
Merged via the queue into cmss13-devs:master with commit 23cd813 Aug 7, 2024
32 checks passed
cm13-github added a commit that referenced this pull request Aug 7, 2024
@Doubleumc Doubleumc deleted the shrapnel-smooth-movement branch August 7, 2024 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Improvement Make the code longer Needs Testing Need to test it on the guinea pigs (production server) Testmerge Candidate we'll test this while you're asleep and the server has 10 players
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants