About the pull request

Refactors launching into a subsystem instead of using sleeps
Also some mini refactors to replace variables with traits and use signals where possible
Precursor to movement system refactor to clean up how collisions are handled and try to standardize them
- Main idea is instead of spamming type checks everywhere until something happens, either the movable atom doing the collision or the atom being collided with will define behavior. Still figuring that out.

Explain why it's good for the game

Don't remember why sleeps aren't good but think a subsystem would be preferable here :P
Tries to simplify launching callback logic by having a single callback instead of a list that needs to be resolved through path matching and conditions (such as call all) that are not super straightforward
General code cleanup where I felt like it

Testing Photographs and Procedure

Mainly tested by throwing things around as a human, testing pounce callbacks from xenos (crusher, runner, hugger), and grenade explosion throws. However this PR is pretty big so think TM would be more sufficient for testing anyways.

n/a

Changelog

:cl: TheDonkified code: Refactors launching into a subsystem /:cl:

Sep 04 '24 06:09 boskoramen

tossing items at other items on the ground causes them to push the other item.

https://github.com/user-attachments/assets/67cbe36b-6c85-4a7a-bd0b-112e5e6932f5

tossing items at tables causes them to "bounce" a tile back as if they hit a wall.

https://github.com/user-attachments/assets/aef57d07-12ee-4d04-9d29-10b0622b0b6c

Sep 23 '24 21:09 private-tristan

tossing items at other items on the ground causes them to push the other item. PVP.CM.Launching.Subsystem.Bug.mp4

tossing items at tables causes them to "bounce" a tile back as if they hit a wall. PVP.CM.Launching.Subsystem.Bug2.mp4

@private-tristan Thanks for this, fixed here: https://github.com/cmss13-devs/cmss13/pull/7110/commits/25f83508dd63e6adfb6ff88a8b8e6b18c98f28b8. As a note though would prefer you open up an issue about this instead of posting directly in PR, helps with visibility of the issue.

Sep 24 '24 03:09 boskoramen

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Oct 06 '24 05:10 cm13-github

Conflicts have been resolved. A maintainer will review the pull request shortly.

Oct 06 '24 07:10 cm13-github

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Oct 09 '24 08:10 cm13-github

Off TM due to conflict, ping someone on discord when fixed.

Oct 09 '24 18:10 realforest2001

Conflicts have been resolved. A maintainer will review the pull request shortly.

Oct 10 '24 04:10 cm13-github

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Oct 15 '24 14:10 cm13-github

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

Oct 27 '24 00:10 cmss13-ci[bot]

Conflicts have been resolved. A maintainer will review the pull request shortly.

Oct 30 '24 04:10 cm13-github

pretty sure this makes warrior lunges way slower, atleast when i tested them it felt way more sluggish, this will probably effect every single throw like lunge, pounces, prae dashes etc

this might just be sensory depravation because its 9 am for me, but code says theyre both SPEED_FAST but SPEED_FAST /feels/ slower

Oct 31 '24 05:10 Red-byte3D

I'm not sure if this is worth doing, or maybe it needs a closer look at the current implementation. At least on local it is less smooth (as in an opressor hook animates a jump 2 tiles, then a jump 2 more tiles; whereas old behavior is a smooth tween), and the speed of it at least on local seemed faster; but its entirely possible on live it gets more delayed by subsystem queueing.

https://github.com/user-attachments/assets/f2a008e3-0608-4688-aa28-be05c273f9b2

Oct 31 '24 08:10 Drulikar

I'm not sure if this is worth doing, or maybe it needs a closer look at the current implementation. At least on local it is less smooth (as in an opressor hook animates a jump 2 tiles, then a jump 2 more tiles; whereas old behavior is a smooth tween), and the speed of it at least on local seemed faster; but its entirely possible on live it gets more delayed by subsystem queueing. Timeline.1.mp4

Imagine this is because in old implementation each movement was executed individually with a sleep in between while with system approach we can process multiple movements at once to compensate for delay between subsystem firing

Old implementation:

var/delay = 10/cur_speed - 0.5 // scales delay back to deciseconds for when sleep is called
...
for (var/turf/T in path)
		...
		if (!Move(T)) // If this returns FALSE, then a collision happened
			break
		...
		sleep(delay)

Subsystem-based implementation:

time_since_last_move += delta_time
var/move_speed = 1 / max(10/launched.cur_speed - 0.5, 0.01)
...
// If we have not moved yet, perform at least 1 move
var/move_count = max(time_since_last_move * move_speed, moved ? 0.0 : 1.0)
...
while (. == LAUNCH_RESULT_SUCCESSFUL && move_count >= 1.0)
    if (!turf_path.len)
        . = LAUNCH_RESULT_STOPPED
        break
    move_count -= 1.0
    next_turf = turf_path[turf_path.len]
    turf_path.len--
    if (HAS_TRAIT_FROM(launched, TRAIT_IMMOBILIZED, BUCKLED_TRAIT))
        . = LAUNCH_RESULT_STOPPED
        break
    if (launched.loc != last_turf)
        . = LAUNCH_RESULT_STOPPED
        break
    if (!launched.Move(next_turf))
        . = LAUNCH_RESULT_STOPPED
        // Thing being launched can be launched again by any collisions from Move() call
        // If there is a desired execution of events (e.g. launch impact called BEFORE new throw), consider
        // changing any calls that launch the object to an INVOKE_NEXT_TICK call
        if (QDELETED(src))
            return LAUNCH_RESULT_DELETED
        break

Can look into animating the movement in between each process call.

Nov 03 '24 01:11 boskoramen

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Nov 11 '24 03:11 cm13-github

i think this is likely still worth working on, but seems to be stale with conflicts for a couple of months. let me know if you're down to work on it again, doing some clientside smoothing would likely fix complaints here

Dec 15 '24 22:12 hry-gh