-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Make radioactive material radioactive #34436
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank for the contribution!
praise: Your PR looks good, and I don't see any issues that would block this code-wise.
issue (blocking): I am wondering the problems that were brought up in #29545 have been addressed.
Bananium used to be radioactive, but it was removed as you could create a pile of them next to a radiation collector for essentially infinite power.
That PR even mentions the lack of uranium's radioactivity, and states that's a good thing, as it would have the same infinite power problem.
thought: Also, is there any thing that may require mappers may need to think about when placing the uranium on a map?
For maps that place it in the engineering vault, they made need to have extra considerations like moving it away from the plasma to minimize radiation damage.
suggestion (non-blocking): It also seems that lighting is inconsistent between the uranium sources. I believe that if it is radioactive, it should glow green as an indicator. Some of the uranium sources already do this, but not all (e.g. the sheets).
question: Also, is there any formula behind the slope values, or was that simply something you tested and worked well?
Thanks!
I will test with this a bit and get back to you. I can scale down the stackable item's radiation values if it turns out to be useful for power.
Engineers have radiation resistant hard suits to slow down the damage, and I also tried to address this using the slope values. Mappers may want to consider moving engineering's uranium into a lead box to neutralize the radiation threat.
I can add this.
I set For |
No free power for engineering
Changes the slope from 3 to 300 on the items, which:
|
And radium (which is a million times more radioactive than uranium) still emits no rads... |
As far as I'm aware, radium currently only exists as a liquid reagent, rather than a solid material. I don't think any liquids passively emit radiation at present, and it doesn't look like this PR changes that. |
Personally I like there being some more dangers to watch out for when mining. And at the moment we don't have a lot of radiation sources. |
you taste metal I tried to base all of these off of symptoms shown by victims that were directly exposed to high dosages of radiation like Cecil Kelley. |
I think that would be better suited for another PR, as it is a more general problem. Glowing should be a small indicator, though the same problem existed for Bananium and the Cancer Mouse, so it's not really a new problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making my requested changes!
praise: I think the changes all look good, and I couldn't create infinite power with the sheets when testing in-game.
question (non-blocking): Should we make uranium glass and its descendants radioactive as well (e.g. uranium shiv)? This should probably be done in a separate PR, as that may have more balance ramifications.
note: And as slarticodefast and ilovehans10 brought up, the lack of warning signs of radiation is quite a problem.
I think that should be done in a different PR as this has been a problem for quite a while and isn't directly related to this change. Things like the Singularity and Cancer Mouse are highly deadly in their radiation damage, yet don't have any direct symptoms.
Just like the other radiation sources, I believe having our current visual indicator (Cancer Mouse glows green, Singularity is the singularity) should be sufficient for now. Plus, the uranium sheets are basically hot potatoes right now and aren't a significant risk unless you carry them around for a good amount of time.
- type: RadiationSource | ||
intensity: 0.1 | ||
slope: 300 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (blocking): Slarti has suggested that we go back to slope 3, as they don't think that it would be too big of a problem with the radiation collectors. Sorry for making you revert that!
Yeah, I guess the warning signs from radiation damage should be done in a separate PR.
|
Yeah, that’s true. I think this would need to be solved as it seems pretty important to not cause unneeded confusion. Currently it’s kept so low as to stop it from generating power using the radiation collectors. The underlying problem wasn’t solved since Bananium’s radioactivity removal. A stack or two can basically power a whole station.
Geiger counters that are on your person will still detect it just fine, and you are in no danger if you are not directly holding them.
I think if we could stop the power generation problem, this would be the appropriate distance. |
I don't think it's that much of a problem in this case since the intensity is relatively low compared to bananium and with all the uranium you could just run a bunch of super pacmans instead, which would also power the whole station. |
True. I think Bananium was intensity 1, while this is only intensity 0.1. A Slope 3 should be fine then, as that seemed to have the desired effect earlier. I'll go look into maybe seeing how we can maybe whitelist radiation sources for powergen as there's also the damaged RTG problem for infinite power. |
This reverts commit 2acbda2
Brought the slope back to 3, if it becomes an issue we can always make a hotfix to bring it up to 50. Optimally we would have 3 different types of radiation (alpha, beta, gamma) and change the effects based on that. However, that is out of scope for this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (blocking): After discussing with some other maintainers, there may be an issue with the fact that the uranium rocks may harm the mobs on the vgroid if they happen to spawn in proximity to it.
This is probably for the best as the current radiation system tries to send rays from the radiation sources to all mobs within 50 tiles, so this could potentially add unnecessary strain to the server.
Slarticodefast suggested that it can just be explained by the rocks shielding the radiation.
suggestion: The solution is just (unfortunately) revert the rock change.
FYI, this inspired me to try some things myself and I can across what seems to be a pretty consistent bug, that if you have intensity set to 0.1, no radiation seems to be generated at all, or at least you take 0 toxin damage and the geiger counter doesn't register anything. #34456 |
I think we've tracked the reason down, though I can confirm that the The solutions would either be to change the space-station-14/Content.Server/Radiation/Systems/RadiationSystem.GridCast.cs Lines 136 to 137 in e094b79
It may be a little out of scope for this no C# PR, but the change seems small enough and is more of a bug-fix that I think it could be fine (though I can't really make any definitive statement). |
While it'd be nice feature, and more realistic, for uranium to be radioactive, I don't think it should be made radioactive, mainly because it'd probably be pretty bad for performance if there's a decent number of sheets and ores around the station (unless its much rarer than I assume it is). It'd probably require a radiation rewrite. Either to make it better handle many sources by using some kind of radiation tilemap instead of iterating over sources & receivers, or at the very least to rewrite so that it no longer assumes that there's more receivers than sources. E.g., at the moment it needs to call A compromise might be to make it radioactive only if it is also unstable and rapidly decays away to effectively limit the total number that can exist at any one time, but that obviously also has significant balance consequences and removes some of the realism. |
Valid consideration. While I think it would make sense for space creatures to have evolved resistance to solar radiation (and by extension what Uranium can put out), I'll leave that for a different place and remove the radiation from the rocks.
I don't see sheets and ores around that often. Engineering always gets a stack at round start, but otherwise it's limited to what salvage can get from mining. And this if anything will encourage people to put uranium into lathes instead of leaving it out to irradiate people.
I vote that C# is out of scope for no C# PRs. |
Update: the code does check for if the ray will have any effect right after it checks the 50 tile distance limit, so it will stop itself from calculating rays needlessly. // get direction from rad source to destination and its distance
var dir = destWorld - sourceWorld;
var dist = dir.Length();
// check if receiver is too far away
if (dist > GridcastMaxDistance) // ← This checks if the entities are more than 50 tiles apart.
return null;
// will it even reach destination considering distance penalty
var rads = incomingRads - slope * dist;
// Apply rad modifier if the source is enclosed within a radiation blocking container
rads = GetAdjustedRadiationIntensity(sourceUid, rads);
if (rads <= MinIntensity) // ← Distance effectiveness check is here. Also slightly incorrect as @Aeshus pointed out.
return null; |
Might be fine since we did not have those problems with bananium either when it was radioactive |
Electro has made a PR for some radiation system changes that should hopefully alleviate some of the performance concerns. |
I didn't take into account that this would be pretty effective at incentivising people to get rid of sheets as quickly as possible, so that would definitely help. I don't really have a feel for how common uranium sheets/ore are, but if they're really not all that commonplace it's probably fine? I just checked midround on lizard, and it seems like there were only 10 so it seems ok
Currently that code is actually abysmally slow for what it should be doing, but that's a separate issue. Though even if if a check is cheap, it can still hurt performance just because it gets run a large number of times, especially if it scales badly like radiation does.
Sure, but, that could still result in a PR being blocked until C# issues get fixed. Though like I said probably fine here. |
is it still dangerous if dragged or shoved in a locker or crate near you? I feel that should probably be a thing |
About the PR
Uranium is now weakly radioactive.
Why / Balance
People expect uranium to be radioactive.
Technical details
Yaml was changed to make uranium and related entities radioactive.
The ore rocks look like they have high intensity in code (2.3), but are actually very low because the rock's radiation shielding weakens it immensely.Media
Requirements
Breaking changes
Changelog
🆑 WarPigeon