Skip to content

Conversation

@bitsandfoxes
Copy link
Collaborator

No description provided.

Comment on lines +13 to +20
StartCoroutine(ResetEffectCooldown(currentEffectCooldownModifier));
}

private IEnumerator ResetEffectCooldown(float cooldownModifier)
{
yield return new WaitForSeconds(_effectDuration);

Player.Instance.WeaponManager.GlobalEffektCooldownModifier = cooldownModifier;
Copy link

Choose a reason for hiding this comment

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

Bug: The BeerbootPickup's coroutine to reset GlobalEffektCooldownModifier is stopped prematurely because its GameObject is destroyed immediately after collection, making the effect permanent.
Severity: CRITICAL

🔍 Detailed Analysis

When a player collects a BeerbootPickup, it sets the static WeaponManager.GlobalEffektCooldownModifier. It then starts a coroutine on its own GameObject to reset this modifier after _effectDuration. However, the base class PickupBase immediately calls Destroy(this.gameObject), which terminates the coroutine before it can complete. As a result, GlobalEffektCooldownModifier is never reset, permanently altering weapon cooldowns for the rest of the game session and breaking game balance.

💡 Suggested Fix

The coroutine should be started on a persistent GameObject, such as the player's GameObject, which is not destroyed after the pickup is collected. This can be achieved by having the OnCollect method call a method on the player object that starts and manages the temporary effect coroutine, similar to how SpeedPickup and DamageResistPickup are implemented.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: Assets/Scripts/Pickups/BeerbootPickup.cs#L13-L20

Potential issue: When a player collects a `BeerbootPickup`, it sets the static
`WeaponManager.GlobalEffektCooldownModifier`. It then starts a coroutine on its own
`GameObject` to reset this modifier after `_effectDuration`. However, the base class
`PickupBase` immediately calls `Destroy(this.gameObject)`, which terminates the
coroutine before it can complete. As a result, `GlobalEffektCooldownModifier` is never
reset, permanently altering weapon cooldowns for the rest of the game session and
breaking game balance.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8393619

@bitsandfoxes
Copy link
Collaborator Author

Superseded by #4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant