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

Eggsac carrier revival #4716

Merged
merged 9 commits into from
Nov 27, 2023
Merged

Eggsac carrier revival #4716

merged 9 commits into from
Nov 27, 2023

Conversation

Birdtalon
Copy link
Contributor

@Birdtalon Birdtalon commented Oct 18, 2023

Forum Thread

About the pull request

The concept of this PR is to find a middle ground between the current eggsac carrier and the pre #4459 eggsac carrier.

This PR will make the following changes. (From this point "normal weeds" can be substituted for "off hive weeds" Placing eggs on hive weeds is unchanged.)

  • Eggsac carrier can once again place eggs on normal weeds.
  • Eggsac carrier can only place 4 eggs at once on normal weeds.
  • If the Eggsac places more than 4 eggs on normal weeds, their oldest placed egg will die. No hugger release.
  • Eggs placed on normal weeds have a maximum lifetime of 5 MINUTES after which they will die. No hugger release.
  • Eggs placed on normal weeds have a 1 MINUTE life whilst the carrier is further than 14 tiles away from them.
  • If the carrier dies, all of their sustained eggs die.

Explain why it's good for the game

Eggsac carrier at the moment is in a bit of a poor place and has gone from being very strong to quite poor. Considering the limitation of having to place only on hive weeds.
The majority of feedback I read against eggsac carrier was with the quantity of eggs able to be placed, as well as the locations they can be placed in, all across the map and with impunity.

This PR aims to address those concerns to make the eggsac both less infuriating to play against while still being satisfying to play as a frontline support or as a stealthy trapper.

Eggs can no longer be placed all over the map because of the 4 egg limit off weeds, so the carrier has to think where they want to impact the map. The carrier also has to stay within a reasonable distance to where they are impacting with their eggs which localises their impact to their immediate play area. The carrier also has to be more reactive to current events as they cannot place an egg which then becomes useful 30 minutes later.

Killing the carrier also has a small reward as in addition to removing a xeno from the game, the eggs they are sustaining are cleared. If a carrier is supporting the front and dies, the marines don't then have to clear X number of eggs AFTER the kill.

Testing Photographs and Procedure

  1. Spawn as egg carrier.
  2. Plant egg on normal weeds.
  3. Move 15+ tiles away.
  4. Wait 1 minute
  5. OR Wait 5 minutes within 14 tiles.
  6. Kill the carrier.

The 5 minute lifetime of the eggs will also clear the eggs in the case that the carrier is admin deleted, or some other weird stuff happens which doesn't result in a death. This will also catch carriers de-evolving

Screenshots & Videos

Put screenshots and videos here with an empty line between the screenshots and the <details> tags.

Changelog

🆑
add: Eggsac carrier can now place eggs on normal weeds to a maximum of 4 eggs.
add: Eggsac carrier eggs on normal weeds have an expiry date.
/:cl:

@Birdtalon
Copy link
Contributor Author

Need to add: handling for eggs bursting for marines or being destroyed as I forgot this. Can't set to draft on mobile github.

@Birdtalon Birdtalon marked this pull request as draft October 19, 2023 06:53
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Oct 27, 2023
@Birdtalon Birdtalon closed this Oct 27, 2023
@Birdtalon
Copy link
Contributor Author

Updated with further handling I missed originally.

@Birdtalon Birdtalon reopened this Oct 31, 2023
@Birdtalon Birdtalon marked this pull request as ready for review October 31, 2023 19:45
@github-actions github-actions bot removed the Stale beg a maintainer to review your PR label Nov 1, 2023
Copy link
Contributor

github-actions bot commented Nov 9, 2023

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

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Nov 9, 2023
@fira fira added the Balance You need to be a professional veteran game maintainer to comprehend what is being done here. label Nov 9, 2023
code/modules/cm_aliens/structures/egg.dm Outdated Show resolved Hide resolved
code/modules/cm_aliens/structures/egg.dm Show resolved Hide resolved
code/modules/cm_aliens/structures/egg.dm Outdated Show resolved Hide resolved
@Zonespace27 Zonespace27 removed the Stale beg a maintainer to review your PR label Nov 10, 2023
code/modules/cm_aliens/structures/egg.dm Show resolved Hide resolved
set_owner(planter)

/obj/effect/alien/egg/carrier_egg/Destroy()
. = ..()
Copy link
Contributor

Choose a reason for hiding this comment

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

call parent at the end of destroy

code/modules/cm_aliens/structures/egg.dm Show resolved Hide resolved
var/datum/behavior_delegate/carrier_eggsac/my_delegate = planter.behavior_delegate
my_delegate.eggs_sustained += src
owner = planter
RegisterSignal(owner, COMSIG_PARENT_QDELETING, PROC_REF(cleanup_owner))
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to do this, you can just do the destroy bit i mentioned above

UnregisterSignal(owner, COMSIG_PARENT_QDELETING)
owner = null

/obj/effect/alien/egg/carrier_egg/proc/cleanup_owner()
Copy link
Contributor

Choose a reason for hiding this comment

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

code doc

var/sustain_distance = 14

/datum/behavior_delegate/carrier_eggsac/append_to_stat()
. = "Eggs sustained: [length(eggs_sustained)] / [egg_sustain_cap]"
Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure it's = instead of +=?

Copy link
Contributor Author

@Birdtalon Birdtalon Nov 11, 2023

Choose a reason for hiding this comment

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

I don't think it actually matters in this case. But comparing it to the same functionality on other behavior delegates I should be consistent with them, also returning it as a list.

@Zonespace27 Zonespace27 added the Balance Approved This PR has had its balance and gameplay-affecting aspects approved. Cry to the Head-maint about it. label Nov 10, 2023
@Zonespace27
Copy link
Contributor

Willing to give this a shot.

@Birdtalon
Copy link
Contributor Author

Think I've addressed all your comments.

  • Remove the signal handler
  • null owner on destroy
  • Added code doc where requested
  • Returning list and += on append_to_stat()

Copy link
Contributor

@Zonespace27 Zonespace27 left a comment

Choose a reason for hiding this comment

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

a few nits left

Comment on lines 310 to 312
var/mob/living/carbon/xenomorph/carrier/owner = null
var/last_refreshed = null
var/life_timer = null
Copy link
Contributor

Choose a reason for hiding this comment

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

code doc these

if(life_timer)
deltimer(life_timer)
owner = null
. = ..()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
. = ..()
return ..()

Comment on lines 51 to 53
var/list/eggs_sustained = list()
var/egg_sustain_cap = EGGSAC_OFF_WEED_EGGCAP
var/sustain_distance = 14
Copy link
Contributor

Choose a reason for hiding this comment

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

code doc

@Birdtalon
Copy link
Contributor Author

Addressed your comments.

Copy link
Contributor

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

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Nov 24, 2023
@Zonespace27 Zonespace27 added this pull request to the merge queue Nov 27, 2023
@Zonespace27
Copy link
Contributor

Sorry for letting this sit for so long! Looks good to go.

@Zonespace27 Zonespace27 removed the Stale beg a maintainer to review your PR label Nov 27, 2023
Merged via the queue into cmss13-devs:master with commit f367771 Nov 27, 2023
26 checks passed
cm13-github added a commit that referenced this pull request Nov 27, 2023
@HeresKozmos
Copy link
Contributor

nice!

@Zonespace27 Zonespace27 mentioned this pull request Nov 28, 2023
@Birdtalon Birdtalon mentioned this pull request Nov 28, 2023
github-merge-queue bot pushed a commit that referenced this pull request Nov 30, 2023
# About the pull request

In #4716 I forgot about hardy weeds. This allows carrier to plant the
fragile eggs on standard and hardy weeds which are basically standard
weeds +.

# Explain why it's good for the game
# Testing Photographs and Procedure
<details>
<summary>Screenshots & Videos</summary>

Put screenshots and videos here with an empty line between the
screenshots and the `<details>` tags.

</details>


# Changelog
:cl:
fix: Eggsac fragile eggs can be placed on hardy weeds.
/:cl:
github-merge-queue bot pushed a commit that referenced this pull request Dec 3, 2023
# About the pull request

So in one of various refactors of #4716 I removed some important code
from `Burst()` for the new egg type which removes it from the carrier's
sustained eggs. This is bad because we keep trying to call procs on eggs
which have already burst etc.

This removes the egg from the carrier's sustain list properly on
Burst().
I also added a check in Life() on the carrier to remove it from their
sustained eggs. If something funky happens we don't want to impact the
player from playing

# Explain why it's good for the game
# Testing Photographs and Procedure
<details>
<summary>Screenshots & Videos</summary>

Put screenshots and videos here with an empty line between the
screenshots and the `<details>` tags.

</details>


# Changelog
🆑 
fix: Eggsack carrier eggs get properly removed from sustained list upon
bursting.
/🆑
github-merge-queue bot pushed a commit that referenced this pull request Dec 9, 2023
# About the pull request
Now that #4716 's been merged,
here's the changes I've wanted to make for eggs for a bit now.

- Eggs can no longer be planted in vehicles
- Eggs can no longer be planted on tiles that would obstruct the egg. 

# Explain why it's good for the game

- In the vast majority of cases, you cannot tell that a vehicle you're
entering is infested with you-instantly-lose eggs. While normally
getting hit with an egg is reasonably predictable (there are weeds ==
possibility of eggs), having no indication of it until you go inside and
get hugged isn't fun or balanced.
- Eggs being able to layer under things like grass is technically a
"feature", but not one I would like to keep. Traps serve the exact same
purpose, but are much harder to mass produce (as intended). Eggs should
be cheaper but more detectable, which doesn't align them being able to
be near-fully hidden.
# Testing Photographs and Procedure
<details>
<summary>Screenshots & Videos</summary>


![image](https://github.com/cmss13-devs/cmss13/assets/41448081/7151c503-72fa-4b1e-acba-576f556420c1)


</details>


# Changelog
:cl:
balance: You cannot plant eggs inside vehicles.
balance: Eggs can no longer be planted on tiles with objects that would
obscure them.
/:cl:
@Birdtalon Birdtalon deleted the eggsacc branch December 21, 2023 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Balance Approved This PR has had its balance and gameplay-affecting aspects approved. Cry to the Head-maint about it. Balance You need to be a professional veteran game maintainer to comprehend what is being done here. Feature Feature coder badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants