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 Subsystem and misc adjustments #1608

Merged
15 commits merged into from
Dec 15, 2022
Merged

Conversation

fira
Copy link
Member

@fira fira commented Nov 18, 2022

About The Pull Request

What this does:

  • Adds a projectile subsystem for scheduling their movement. The follow_flightpath sleeps are detrimental to perf, and instead this allows them to be updated 2x as often.
  • Projectile speed is +-15% and has a firing offset, to result in a proper bullet trail instead of a dotted line.
  • Gun projectile firing angle takes into account scatter and clicked pixel. This is mostly a visual change, but also affects direction bullets take past initially aimed point.
  • Decouples gun/projectile from sleeps. This has caused many practical issues in the past.
  • Removes simple_mobs firing and AMMO_SCANS_NEARBY. They're defunct/broken.
  • Removes projectile speed-up from AMMO_ROCKET - instead granting them full speed. This only really mattered for thermobaric launcher at long range.

Why It's Good For The Game

Smoother, more visual feedback (perhaps to a fault, as it shows how goofy our system is)

Example of in-turf targeting, two trajectories based on clicking either X mark:
image

Video example of difference, yes, it is subtle:
https://streamable.com/ph18yj
https://streamable.com/ndq7f5

Changelog

🆑
add: Added +-15% variation to bullet speed and firing offsets to result in a better bullet trail.
add: Gun firing now takes into account click position. This can bend the bullet trajectory beyond the clicked point.
add: Projectiles now attempt to mimick a proper bullet stream by offsetting slightly. This is purely visual.
code: Removed rocket speed-up, and gave them full speed. Only really matters for offscreen admin-spawned thermobaric rockets.
fix: Fixed deletion issues with projectiles, and decoupled it from gun logic due to recurring issues.
del: Removed simple_mob-s shooting and AMMO_SCANS_NEARY. They're defunct/broken.
fix: Removed a number of micro sleeps in explosion code. This may reduce amount of shrapnel hits.
code: Tweaked Aimed Shot homing logic which should make it even more accurate in the offchance it ever matters.
/:cl:

@github-actions github-actions bot added Feature Feature coder badge Fix Fix one bug, make ten more labels Nov 18, 2022
Copy link
Member

@hry-gh hry-gh left a comment

Choose a reason for hiding this comment

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

this certainly looks good for a TM - i'm sure there's edge cases that i can't predict rn that'll come up then

@fira fira added the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Nov 20, 2022
@ManoPablo

This comment was marked as off-topic.

@github-actions github-actions bot added the Merge Conflict PR can't be merged because it touched too much code label Nov 23, 2022
@github-actions github-actions bot removed the Merge Conflict PR can't be merged because it touched too much code label Nov 23, 2022
@fira
Copy link
Member Author

fira commented Nov 23, 2022

Rocket speedup was super painful and unreliable to deal with when it came to variable speed and travel carryover so i just removed it entirely, giving them full speed all the time instead...

This impacts two situations:

  • It makes the RPG hit more reliably and slightly faster beyond PB range (max 15-20%) which should fall back in line with people expectations from before
  • It makes admin-spawned thermobaric launcher faster without having to fire offscreen for it, but that's basically never used

@github-actions github-actions bot added Merge Conflict PR can't be merged because it touched too much code and removed Merge Conflict PR can't be merged because it touched too much code labels Nov 23, 2022
@fira
Copy link
Member Author

fira commented Nov 26, 2022

The sleep safety net isn't good enough, causing erroring bullets to be stuck in midair. This is a problem because it appears we still have sleeps in /mob/living/bullet_act somehow (i suspect heavily /mob/living/carbon/Xenomorph/apply_damage)

@github-actions github-actions bot added the Merge Conflict PR can't be merged because it touched too much code label Nov 28, 2022
@github-actions github-actions bot removed the Merge Conflict PR can't be merged because it touched too much code label Nov 28, 2022
@fira fira requested a review from Segrain November 30, 2022 06:50
Copy link
Member

@hry-gh hry-gh left a comment

Choose a reason for hiding this comment

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

code quality lgtm, still possible there's something being overlooked, but

@github-actions github-actions bot added the Merge Conflict PR can't be merged because it touched too much code label Dec 7, 2022
@github-actions github-actions bot removed the Merge Conflict PR can't be merged because it touched too much code label Dec 9, 2022
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Code LGTM. Testmerges went well.

@ghost ghost merged commit fcd0e11 into cmss13-devs:master Dec 15, 2022
github-actions bot added a commit that referenced this pull request Dec 15, 2022
github-actions bot added a commit that referenced this pull request Dec 15, 2022
@Doubleumc Doubleumc mentioned this pull request Nov 21, 2023
6 tasks
github-merge-queue bot pushed a commit that referenced this pull request Oct 28, 2024
# About the pull request

Fixes Issue #3053 (and potentially #1764)
This PR aims to fix a bug that has been around for a long time which is
explained below.
Changes tested.

The reason why buckshots and any other multi-projectile weapons (or
explosions) are not working correctly is because of the PR that added
random speed variance for projectiles (#1608). This PR caused the
buckshot's additional projectiles to either lag behind or go faster than
the main projectile which stuns the target, causing the additional
projectiles to not hit the target most of the time.
However there is also another factor which causes the additional
projectiles to not hit the target. That is the fact that when the main
projectile hits and stuns the target (makes them fall down to the
floor), the additional projectiles miss the target because they are on
the floor and go over the target.
This causes the shotgun to not deal the intended damage because only the
main projectile hits and barely does any damage.

**This is a bugfix. It doesnt buff the projectile damage in any way. It
technically also only affects stunnable targets because the problem is
targets being stunned and the projectiles missing them.
For example; Hivelords dont get stunned, therefore they dont have an
issue with the additional projectiles.**

# Explain why it's good for the game

A buckshot shouldn't miss a target that you are adjacent to.
The bug causes the buckshot to be completely useless at times, because
it doesnt work as intended.
Even though you are adjacent to your target, the projectiles miss the
target which clearly shouldn't happen.
If you are still confused as to what this PR fixes exactly, videos below
should show the problem.

# Testing Photographs and Procedure

**These are before / after footages. First video is before the fix,
second video is after the fix.**
<details>
<summary>Screenshots & Videos</summary>

Before


https://github.com/user-attachments/assets/364bb19e-25a5-4f6f-9dd3-ce4beea0fcfa

After


https://github.com/user-attachments/assets/b797c3b3-97d2-476b-bac6-f450b976efe6



</details>


# Changelog
:cl: Ansekishoku
fix: Buckshot additional projectiles no more miss the target when they
get stunned by the main projectile.
/:cl:

---------

Co-authored-by: Doubleumc <Doubleumc@users.noreply.github.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Feature coder badge Fix Fix one bug, make ten more Testmerge Candidate we'll test this while you're asleep and the server has 10 players
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants