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

Projectile smooth movement #4986

Merged
merged 8 commits into from
Dec 7, 2023

Conversation

Doubleumc
Copy link
Contributor

@Doubleumc Doubleumc commented Nov 21, 2023

About the pull request

Changes the projectile's visual position to animate() across its projected path, providing smoother visuals, especially at higher client framerates.

Bresenham
Black squares: tiles actually traversed by the projectile object
Red line: visual path taken by the projectile
Purple marks: equidistant marks corresponding to number of traversed tiles (black squares)
Blue marks: closest points to center of traversed tiles (black squares)

The projectile's physical path (black squares) is unchanged. The system calculates how far along the physical path the projectile has travelled, draws a line from the start point to the end point (red line), then interpolates where along that line corresponds to that distance travelled (purple marks). This is slightly less accurate to the projectile's physical position than the closest point of the line (blue marks), but this slight potential offset is preferred for its smoother visuals.

This does not touch any code that actually effects the bullet's trajectory, this is purely cosmetic.

Tested:

  • handheld guns
  • scatter
  • in-turf targeting (as visually consistent as the current implementation, at least)
  • rockets
  • plasma caster
  • xeno spit

Explain why it's good for the game

Smoother apparent movement for projectiles gives a more appealing look.

Testing Photographs and Procedure

Screenshots & Videos

https://streamable.com/h6esir

Changelog

🆑
code: projectiles smoothly animate their movement
/:cl:

@github-actions github-actions bot added the Code Improvement Make the code longer label Nov 21, 2023
@Nanu308
Copy link
Member

Nanu308 commented Nov 21, 2023

Just a question, would this have any performance impact at all?

@fira
Copy link
Member

fira commented Nov 21, 2023

THANK YOU SO MUCH i tried to get this right several times when rewriting bullet travel but I simply couldn't manage to and gave up

An undesirable side effect of this is that instead of skimming walls the bullets will cut through corners but i'm hoping this is visually acceptable

Did you test this with M2C too? With current botched system the M2C bullets have an erratic trajectory for some reason, i suspect something is different with it

This probably has some performance impact clientside but given it's only translating with pixel_x/y and there's no transforms at work I'd think it's nothing major

@fira fira added the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Nov 21, 2023
@Doubleumc
Copy link
Contributor Author

Just a question, would this have any performance impact at all?

Yes, but to what degree I don't know. There's the math, and the animate(). The math is basically a wash, as it does more operations but only once at the end of movement, instead of for every tile of movement. Its simple arithmetic in either case, nothing fancy.

The animate() is the elephant in the room. I understand this is a heavier operation, but to what degree I don't have the experience to say. I'll defer to those more knowledgeable than me.

TGMC, for instance, probably knows a thing or two. They also use an animate() for projectiles under certain circumstances, and had this to say about it:

Changing the vars through animation alone takes almost 5 times the CPU than setting them directly.
https://github.com/tgstation/TerraGov-Marine-Corps/blob/master/code/modules/projectiles/projectile.dm#L571

Lummox, the BYOND dev, also knows a thing or two. In a thread suggesting the same thing as TGMC's comment, he disagreed:

animate() will basically take the same amount of work as setting all those vars at once, but with small overhead
https://www.byond.com/forum/post/1897218#comment15961196

Whether the performance tradeoff is acceptable is something that can only be answered by large-scale testing.

@fira
Copy link
Member

fira commented Nov 21, 2023

I don't think there's going to be any problem performance wise but I really want to see it at work live and ensure it still works well under server load

@Doubleumc
Copy link
Contributor Author

Doubleumc commented Nov 21, 2023

An undesirable side effect of this is that instead of skimming walls the bullets will cut through corners but i'm hoping this is visually acceptable

The quirks of tile-based projectile movement will definitely be more visible as the animation lets you see the whole path. As you said in your original implementation #1608 "Smoother, more visual feedback (perhaps to a fault, as it shows how goofy our system is)"
This is the worst possible case, going from halfway point to halfway point:
image

Did you test this with M2C too? With current botched system the M2C bullets have an erratic trajectory for some reason, i suspect something is different with it

I didn't think to try these before but just did. Bullets come out and go in straight lines.

Copy link
Member

@fira fira left a comment

Choose a reason for hiding this comment

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

Works fine in TM. There is a slight discrepancy in low angle trajectories as the bullet skims despite being angled, but it's still way better than it used to be

@Doubleumc
Copy link
Contributor Author

Works fine in TM. There is a slight discrepancy in low angle trajectories as the bullet skims despite being angled, but it's still way better than it used to be

Could you elaborate on what you mean by this?

@fira
Copy link
Member

fira commented Nov 23, 2023

Works fine in TM. There is a slight discrepancy in low angle trajectories as the bullet skims despite being angled, but it's still way better than it used to be

Could you elaborate on what you mean by this?

I'm not 100% sure but from what I've seen (which was a thing with normal system but i think due to my math being off) some bullets appear especially in low angle trajectories to keep skimming with the angle not being on par with the actual traveling trajectory

It could be something i've just imagined, haven't seen it happen much and bullets fly pretty fast

@restedwaves restedwaves mentioned this pull request Nov 26, 2023
3 tasks
@BadAtThisGame302 BadAtThisGame302 mentioned this pull request Nov 26, 2023
3 tasks
@zzzmike
Copy link
Contributor

zzzmike commented Dec 4, 2023

Is it possible that this is causing really weird disabler issues? I've been noticing disabler shots disappearing and having odd movements a lot more than before this was TM'd. Here's a video, I can't be sure it's a bug or not, but wanted to share https://www.youtube.com/watch?v=ObALzeO-xIo&ab_channel=MK

@Doubleumc
Copy link
Contributor Author

The starting position of the bullets is incorrect with this PR resulting in bullets starting from behind your mob:

Thanks for letting me know! Starting in the firer's turf is as-per the movement code, but it visually appearing there is definitely unintended.

Is it possible that this is causing really weird disabler issues? I've been noticing disabler shots disappearing and having odd movements a lot more than before this was TM'd. Here's a video, I can't be sure it's a bug or not, but wanted to share https://www.youtube.com/watch?v=ObALzeO-xIo&ab_channel=MK

Is definitely possible its causing the odd movements. Or more accurately, its making the odd movements noticeable. The underlying movement code is untouched, its just the visuals that were changed. I'll look into smoothing out those doglegs.

@Doubleumc
Copy link
Contributor Author

Projectile visuals will no longer overlap the firer. Projectile starts transparent and will fade-in based on distance travelled.

Projectile visual path now avoids doglegs. By necessity this slightly decouples the visual path from the real path, so corner-cutting may be slightly more noticeable.

@Drulikar Drulikar added this pull request to the merge queue Dec 7, 2023
Merged via the queue into cmss13-devs:master with commit bdbca13 Dec 7, 2023
26 checks passed
cm13-github added a commit that referenced this pull request Dec 7, 2023
@Doubleumc Doubleumc deleted the projectile-smooth-movement branch December 8, 2023 01:31
@Doubleumc Doubleumc mentioned this pull request Dec 8, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 19, 2023
# About the pull request

Forgot to clear a reference I set in
#4986 and cleared some others
I saw.

Not familiar enough with the Destroy/QDEL pipeline to know what else, if
anything, should be done with `weapon_cause_data` and `bullet_traits`
since they hold references themselves.

<!-- Remove this text and explain what the purpose of your PR is.

Mention if you have tested your changes. If you changed a map, make sure
you used the mapmerge tool.
If this is an Issue Correction, you can type "Fixes Issue #169420" to
link the PR to the corresponding Issue number #169420.

Remember: something that is self-evident to you might not be to others.
Explain your rationale fully, even if you feel it goes without saying.
-->

# Explain why it's good for the game

I will clean up after myself.
# Testing Photographs and Procedure
<details>
<summary>Screenshots & Videos</summary>

Put screenshots and videos here with an empty line between the
screenshots and the `<details>` tags.

</details>


# Changelog
No player-facing changes.
github-merge-queue bot pushed a commit that referenced this pull request Aug 7, 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](https://github.com/user-attachments/assets/c1aea971-6f75-40b8-b34e-985bd57e27af)

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?

<!-- Remove this text and explain what the purpose of your PR is.

Mention if you have tested your changes. If you changed a map, make sure
you used the mapmerge tool.
If this is an Issue Correction, you can type "Fixes Issue #169420" to
link the PR to the corresponding Issue number #169420.

Remember: something that is self-evident to you might not be to others.
Explain your rationale fully, even if you feel it goes without saying.
-->

# Explain why it's good for the game

Smoother apparent movement for shrapnel gives a more appealing look.
# Testing Photographs and Procedure
<details>
<summary>Screenshots & Videos</summary>

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](https://github.com/user-attachments/assets/21e78b1c-1dcb-4181-8b1e-4385498465fd)

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](https://github.com/user-attachments/assets/840ba919-84bf-4d7a-9732-43c6a996b8ac)

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](https://github.com/user-attachments/assets/00016b04-47b5-47f2-8be6-1a7295fb6cf4)

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](https://github.com/user-attachments/assets/cdc95284-59b0-445a-abb0-09b32a69864f)

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.

</details>

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
:cl:
code: shrapnel smoothly animates their movement
code: visual-effect-only shrapnel replaced by clientside particles
/:cl:
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 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.

6 participants