various minor /tg/ bugfixes #2505
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## About The Pull Request Actions that don't give the user control (so don't give them an action button) will now no longer give them to ghosts either. Ghosts should see the same information as the player when observing them. They don't need to see guardian's protection mode and bileworm's spitting, for example. ## Why It's Good For The Game Explained in the about the pull request already, ghosts should have the same information as the player they are orbiting, not see the hidden actions. It makes it annoying for contributors to have to manually set every ability meant to not be seen by players to also not be seen by ghosts. ## Changelog :cl: fix: Action abilities hidden from players are now not shown to observers either. /:cl:
## About The Pull Request Fixes this  When you drag a screen element across empty space that isn't over another screen element - Fixes #84045 ## Changelog :cl: fix: You can move around ui buttons in your action bar /:cl:
…4618) ## About The Pull Request This is a weird one. When `EMISSIVE_BLOCK_UNIQUE` is set for an atom, it causes the emissive blocker to be placed in the atom's `contents`. I don't think this was intended can can potentially cause all kinds of issues like in the linked one.  @LemonInTheDark what say you? Fixes Skyrat-SS13/Skyrat-tg#20431 ## Why It's Good For The Game Bug fix? ## Changelog :cl: fix: fixes emissive blockers sometimes being put in an atom's contents /:cl:
## About The Pull Request Fixes this  Caused by the highlighted line  when switching modes the glasses `color_cutoffs` becomes an empty list, not null When glass color is yellow  When it's blue or off  So, blending an empty list causes the exception ## Changelog :cl: fix: runtime when toggling engineering meson scanners /:cl:
…ring area editing with station blueprints (#75146) ## About The Pull Request Fixes a tiny mistake i made in #73850 which caused a hell a lot of signal override runtimes during area editing with station blueprints. It happens allow it. ## Changelog :cl: fix: signal override & other area sensitive runtimes during area editing with station blueprints /:cl:
…again (#75161)
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may
not be viewable. -->
## About The Pull Request
I swear i didnt fail at this like 3 times i tested it this time.
adds a descriptive error of what spatial grid cells a movable is stuck
in, and in what channels. This only runs during unit tests. hopefully
this should be enough information to go off of to fix the spurious
cockroach error. if its not then i can try tracking all grid cell
changes during unit tests.
error looks like this:
```
[2023-05-03 04:16:34.009] runtime error: /mob/living/trolls_the_maintainer instance, which is in nullspace, and thus not be within the contents of any spatial grid cell, was in the contents of 2 spatial grid cells when it was only supposed to be in one! within the contents of the following cells: {(221, 119, 11), within channels: hearing}, {coords: (136, 136, 14), within channels: hearing}. (code/controllers/subsystem/spatial_gridmap.dm:581)
```
for something located in nullspace but still in the contents of >0 cells
and:
```
runtime error: /mob/living/trolls_the_maintainer instance, which is supposed to only be in the contents of a spatial grid cell at coords: (136, 136, 14), was in the contents of 6 spatial grid cells when it was only supposed to be in one! within the contents of the following cells: {(68, 153, 2), within channels: hearing}, {coords: (221, 170, 3), within channels: hearing}, {coords: (255, 153, 11), within channels: hearing}, {coords: (170, 238, 13), within channels: hearing}, {coords: (204, 119, 14), within channels: hearing}, {coords: (136, 136, 14), within channels: hearing}.
```
if its not in nullspace but its within more than 1 grid cell.
the coordinates here are translated from the index of the given cell to
world coordinates.
## Why It's Good For The Game
mothblocks has been standing outside my house for weeks i am fearing for
my life
---------
Co-authored-by: Mothblocks <[email protected]>
## About The Pull Request There is a check in `Moved` that calls `update_gravity` if the mob is being nullspaced or de-nullspaced. However mobs are nullspaced when being qdeleted. This causes runtimes because mobs that have gone through Destroy and have cleaned up vars (such as DNA and species) are having their gravity updated, and most gravity update logic assumes the mob is in a valid, not qdelling state.  So, `update_gravity` is not called if the mob is being qdelled when moving into nullspace. ## Why It's Good For The Game Runtimes ## Changelog :cl: Melbert fix: Runtimes when deleting humans relating to gravity /:cl: Co-authored-by: san7890 <[email protected]>
…ot] to a qdeleting parent of type [/mob/living/carbon/human]" runtime (#75359) https://github.com/tgstation/tgstation/blob/a1a2b0fa9074069ea1048faa5e503ddb1f63ed04/code/modules/mob/living/carbon/death.dm#L1-L19 The mob can start qdeling @ L12 / in living/death() (if say, for example, they had an explosive implant. ) This is accounted for in human/death(), but someone forgot it here. https://github.com/tgstation/tgstation/blob/a1a2b0fa9074069ea1048faa5e503ddb1f63ed04/code/modules/mob/living/carbon/human/death.dm#L20-L34
…5357) ## About The Pull Request Forgot some arguments to this call to `alert_owner`. ## Why It's Good For The Game Runtimes ## Changelog :cl: Melbert fix: Applying fines via sechud apply correctly /:cl:
## About The Pull Request Meters can have a null target if the pipe below them is destroyed or removed; or if mappers fuck up. ## Why It's Good For The Game Prevents runtimes ## Changelog
## About The Pull Request exceptions need to be thrown --------- Co-authored-by: LemonInTheDark <[email protected]>
This runtiming is bad
## About The Pull Request The issue was map verification calling build_cache, which uses the define which enables/disables init values on sleep. We avoid this by using a var on map datums and using that to enable the init value modification only when we are actually loading stuff. Also fixes a bug in clear_tracked_initialize() where it being called with no values lead to bad values/potentially overriding initialized on accident. Also also I forgot how for loops worked so this would not have worked regardless ## Why It's Good For The Game Code should like, function
…webs are destroyed by hand (#76255) ## About The Pull Request **Reproduction** - Spawn a spider web - Try destroying it with a wire cutter or any other object, but it must be by hand - If you are lucky (50% probability as the web uses the prob() proc) at the moment the web is destroyed a balloon alert at the same time "stuck in web" gets called causing the runtime because it added a timer on the deleted spider web **Solution** Use loc for balloon alerts as that does not get deleted. ## Changelog :cl: fix: fixes balloon alert runtime when spider webs are destroyed. /:cl:
…(#76275) This being 2 is causing all connected clients to get all generated or runtime created assets (like tts), but it is not set to 2 on prod because the rsc cdn requires setting this to 0 via `server_side_modifications.dm` also causes spritesheet to send resources out to all connected clients every insert mid generation, massively slowing it down as it will FUCKING BLOCK during this.
## About The Pull Request  ## Why It's Good For The Game Flaky failure ## Changelog :cl: Melbert fix: Fixes a runtime from people with aphasia trauma getting deleted /:cl:
## About The Pull Request Adds a user check to a line and makes it early return now. ## Why It's Good For The Game Thrown items don't always have a mob throwing them. ## Changelog :cl: fix: Fixed snatcherprods potentially giving held objects a one-way ticket to nullspace if thrown at someone by something that's not a mob. /:cl:
…es (#76764) bug. EDIT: Just kidding, that turned out to not be it either. The issue: During init, when the gravity generator is being set up and creating its proximity field, a turf can get the `COMSIG_ATOM_HAS_GRAVITY` signal registered to it multiple times. It seems like it shouldn't be possible for this to happen due to the `HAS_TRAIT(TRAIT_FORCED_GRAVITY)` check, but it can. I've only seen this happen during CI and have not been able to reproduce it during runtime, but it comes up often enough to be a nuisance when testing PRs. As seen below, causing CI failures. The problem turf is directly below the `gravity_generator/main` object.  I spent too much time trying to figure out the cause of this duped signal when it really does not matter if this signal gets overridden here, since it's always going to be from the same proximity field. Suppressing the warning will stop the CI failures without any ill effects in this case. So let's just do that. Less CI failures for something trivial. :cl: fix: fixes gravity generators causing CI failures from overriding a signal /:cl:
Discovered while working on #76663 ## About The Pull Request We were getting undeleted SQL stack traces with the following: `[2023-07-25 19:27:33.832] DEBUG-SQL: Undeleted query: "SELECT 1 FROM player WHERE ckey = :ckey" LA: NextRow LAT: 39397` There's only one spot in the code (which happens to be the newest, introduced in #74496 / 8fc56cb) where we don't always qdel the query regardless of the path the proc takes (and that seems to match the error per the `LA` field), so let's make sure we always clean that up since it's a drather silly runtime. It's also rather spontaneous, as this code is only executed when servers are bunkered up (which Sybil presently is) This is my first time dealing with SQL Query code, so let me know if I'm missing something critical here.
… CRASHES) (#77309) ## About The Pull Request Servers are crashing on every round restart and I have absolutely no idea where to start, but I noticed this so I figure I'll throw up a PR to fix it. This is the runtime (only found in dd.log, sorry non-admin/maintainer gamers https://[tgstation13.org/raw-logs/sybil/data/logs/2023/08/01/round-211577/dd.log](https://tgstation13.org/raw-logs/sybil/data/logs/2023/08/01/round-211577/dd.log) ) ```txt [00:07:07] Runtime in code/modules/logging/log_holder.dm,166: Attempted to call shutdown_logging twice! proc name: shutdown logging (/datum/log_holder/proc/shutdown_logging) src: /datum/log_holder (/datum/log_holder) call stack: /datum/log_holder (/datum/log_holder): shutdown logging() shutdown logging() world: Reboot(0, 0) Ticker (/datum/controller/subsystem/ticker): Reboot("Round ended.", "proper completion", 600) ``` This is the full log:  This is the code:  For some reason, even though we invoke `TGSEndProcess`, we still continue on in this `if()` chain. I don't know why we keep executing DM code after TGS is supposed to be shut down (which may be why no one has ever included a `return` here, but let's be safe instead of sorry. If you really want to investigate why TGS is running DM code after we end the process, I am amenable to including a stack trace or crash of this phenomenon instead. ## Why It's Good For The Game Since we aren't invoking `LOG_CLOSE_ALL` to rust-g twice (which seems to be really unwanted per the code I read), hopefully thing no crash? Rust-g had two breaking changes (one with logs, and one with SQL), so I'm presuming that this might be related to the log one (which is why we didn't see this sorta thing happen pre-#77307)... Worst case scenario less runtimes in the funny runtime log. I hope this wasn't loadbearing either... Likely requires testmerging since TGS and I don't get along on my machine. ## Changelog :cl: server: Added a preventative measure to prevent calling both TGSHardRestart and TGSReboot, as well as potentially invoking sensitive procs that are only meant to be called once. /:cl: TL;DR- I do not definitively know why servers are crashing but I noticed this runtime so I'll put out this open flame while I have the time to do so.
dwasint
added a commit
that referenced
this pull request
Aug 22, 2024
Absolucy
added a commit
to Absolucy/Monkestation
that referenced
this pull request
Aug 24, 2024
…kestation#3038)" This reverts commit 56fac5f.
dwasint
pushed a commit
that referenced
this pull request
Aug 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
at this point i'm getting too tired to copy/paste every PR link, especially when they're in the damned name of each commit anyways, so, yeah.