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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ports TG glowshrooms #15475

Closed
wants to merge 5 commits into from
Closed

Conversation

Gatchapod
Copy link
Contributor

What Does This PR Do

Alternative to #15445

Ports:
tgstation/tgstation#51023
tgstation/tgstation#52120
tgstation/tgstation#52169
tgstation/tgstation#53652

Outside of that, makes glowshrooms incapable of growing on lava and in water, which was already part of TG code and made enough sense to put in. Also, made porting simpler.
Basically the entire credit goes to the authors of their respective PRs.

Why It's Good For The Game

So, in #15445 Fox convinced me this should be done as an at least a potential alternative. Perhaps it would even be healthier to try this than outright jumping to total removal of a feature.

I would still rather see it removed than reworked, but more than anything I want something to be done about glowshrooms. Knowing well that removal of features is rather controversial and is likely to be rejected or limbo'd, this should be far more acceptable alternative.

Bonus points, it adds a lot of great commentary to the file. Credits to the respective PR authors, again.

My tests on a single node reached up to 7th generation of glowshrooms and total extinction within 15 minutes, which seems acceptable.

Changelog

馃啈
add: Glowshrooms and their mutations will now decay, preventing single node from overtaking the entire station
/:cl:

@AffectedArc07 AffectedArc07 added Feature This PR is a new addition to the game Port This PR is from another server, or heavily inspired/adapted. labels Feb 8, 2021
@DeveloperLoser
Copy link
Contributor

I honestly think now that we should just remove spreading, it takes away nothing as shrooms can still be decoratively used (like intended) and used for R&D, this just makes it easier for a botanist to still run around with a pack full of shrooms, like in the other pr, except these will still spread and just need replanting at some point, i would say do both decay and no spread but that would be annoying for trying to use them decoratively.

(Theres my opinion, if it matters)

@Gatchapod
Copy link
Contributor Author

I'm pretty certain opinions of people will be taken into consideration here. I myself am very curious about them.
So far, removal has gathered sizable amount of thumb ups, but I can already see people who are fine with both solutions, as long as something is done about glowshrooms.
So I do encourage voting as you see fit. I definitely won't hold it against anyone who dares to dislike either (or possibly both!) solutions.

Copy link
Contributor

@Kyep Kyep left a comment

Choose a reason for hiding this comment

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

I still prefer #15445 over this.
There's no good reason for glowshroom spreading to exist.

EDIT March 18th: I've decided to make this an actual objection, not just a comment. I really still prefer #15445 over this.

@Fox-McCloud
Copy link
Member

I've been keeping a very close eye on what TG has done with this; they nerfed them yet again--mind checking out that rework; it makes them even weaker than this PR and solves some performance issues with them.

Copy link
Contributor

@dearmochi dearmochi left a comment

Choose a reason for hiding this comment

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

Lots of lowerCamelCase variables in the added code. Please change them to snake_case for consistency

code/game/objects/effects/glowshroom.dm Outdated Show resolved Hide resolved
code/game/objects/effects/glowshroom.dm Outdated Show resolved Hide resolved
code/game/objects/effects/glowshroom.dm Outdated Show resolved Hide resolved
code/game/objects/effects/glowshroom.dm Outdated Show resolved Hide resolved
Comment on lines +108 to +109
var/chance_stats = ((myseed.potency + myseed.endurance * 2) * 0.2) // Chance of generating a new mushroom based on stats
var/chance_generation = (100 / (generation * generation)) // This formula gives you diminishing returns based on generation. 100% with 1st gen, decreasing to 25%, 11%, 6, 4, 2...
Copy link
Contributor

Choose a reason for hiding this comment

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

These could probably be moved out of the loop since they don't need recalculation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

chance_stats does need recalculation, because in the same loop we call proc/Decay which lowers endurance. I think leaving chance_generation next to it for readability might be worth those few recalculations.

code/game/objects/effects/glowshroom.dm Outdated Show resolved Hide resolved
code/game/objects/effects/glowshroom.dm Outdated Show resolved Hide resolved
code/game/objects/effects/glowshroom.dm Outdated Show resolved Hide resolved
code/game/objects/effects/glowshroom.dm Outdated Show resolved Hide resolved
code/game/objects/effects/glowshroom.dm Outdated Show resolved Hide resolved
@Gatchapod
Copy link
Contributor Author

I've been keeping a very close eye on what TG has done with this; they nerfed them yet again--mind checking out that rework; it makes them even weaker than this PR and solves some performance issues with them.

Porting most recent /tg/ changes would require, at the very least, porting their cooldown systems beforehand.
On paper, those changes look amazing, especially the much nicer diminishing returns curve. I couldn't, however, really test them in practice on a /tg/ build, because they just spew out runtimes and refuse to spread.

One way or another, this is likely as much as we can port from /tg/ regarding glowshrooms, at least for now.

@Gatchapod Gatchapod closed this Mar 13, 2021
@Gatchapod Gatchapod reopened this Mar 13, 2021
@variableundefined
Copy link
Contributor

No head approval on this one. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature This PR is a new addition to the game Port This PR is from another server, or heavily inspired/adapted.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants