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

Add disconnection share and ACU recall options #5971

Open
wants to merge 63 commits into
base: develop
Choose a base branch
from

Conversation

lL1l1
Copy link
Contributor

@lL1l1 lL1l1 commented Mar 13, 2024

It can be game-ruining to lose players because of disconnects, blowing up all the engineers in their base or units at their front and destroying all their units in share until death games. Especially because of recent connection issues/inherent Supcom connection issues. This PR fixes that with two new settings:

  • DC Share Conditions: Determines how units are shared in case of a disconnect. To prevent abuse in Assassination, one ACU for that player must be at full HP and shields for the disconnect share conditions to apply. Default: Same as share condition
    Common use case: share until death players could use fullshare for unfortunate disconnects
  • Recall Disconnected ACUs: Instead of exploding, an ACU can recall and leave everything intact. To prevent abuse, the ACU must be at full HP and shields. This works individually on all ACUs a player owns. Default: Off
    Common use case: could be used in team matchmaker to prevent games from being completely lost due to an early disconnect.

The full HP/shields threshold can be adjusted rather easily. Another idea is to not have enemy combat units within a large range. This would really ensure that the ACU is out of danger and the player isn't just abusing the feature.
Maybe the Recall Disconnected ACUs feature could use a time limit after which the rule goes away?
There is also an idea to leave the ACU around on disconnect, and apply fullshare rules until the ACU is killed, and applying the share condition rules then.

@Garanas
Copy link
Member

Garanas commented Mar 14, 2024

What if instead of checking whether the unit is full health, we instead check if the unit received damage in the last 10 seconds?

We could keep track (for all units) in what game tick that unit received damage for the last time.

@lL1l1
Copy link
Contributor Author

lL1l1 commented Mar 15, 2024

Doesn't updating a value every time any unit takes damage just to check it once at the end of the game sound expensive? I'm not experienced, so this is more of a curiosity/learning question. I do agree it's better for ACUs, since regen/shields varies a lot between factions, and the feature shouldn't depend on faction or upgrade choice.

@Garanas
Copy link
Member

Garanas commented Mar 15, 2024

Doesn't updating a value every time any unit takes damage just to check it once at the end of the game sound expensive?

Yes, as long as the complexity is O(1) then we're fine. There's a lot that happens when units take damage (health adjustments, veterancy) and that runs fine at the moment too.

We could also use the same value to 'enhance' the CTRL + K ability - if a unit did not receive damage the past 10 seconds then it can be destroyed without triggering the death weapon for example.

Note that the '10 seconds' is arbitrarily chosen, can be any value.

@apollodeathstar
Copy link
Contributor

apollodeathstar commented Mar 17, 2024

I feel the easiest choice is to just have the ACU be handed over along with the other units, if it was going to die it would still die anyway + more likely due the the "stall" that happens to units during the handover. This prevents abuse and a 99% health ACU getting shot by a single enemy tank on DC exploding and leading to the same outcome as now (which we are trying to avoid)

I would say either hand the ACU over or make the ACU neutral (civilian) and give it a "recall" CD timer of 30 seconds to 1 min after which it recalls off the field. (despawns)

Also early game the ACU esp if upgraded is a large mass investment from the player that DC'ed. Imagine if you did not get half the Factories or mexs from your allies because they DC'ed. The ACU early game is a small army onto itself.

So I feel the ACU should just be transferred to the remaining players, as they are still being punished for the DC'ed players DC if they do not get it via the loss in combat power.

@lL1l1
Copy link
Contributor Author

lL1l1 commented Mar 17, 2024

I feel the easiest choice is to just have the ACU be handed over along with the other units

they are still being punished for the DC'ed players DC if they do not get it via the loss in combat power.

In a lot of maps and at increasing rating the shared economy and units can be more beneficial than the control or commentary from another player, enough to overcome the loss of an ACU. It has also been brought up on Discord that sharing ACUs would be extremely powerful (two gun coms perfectly coordinated).

make the ACU neutral (civilian)

This relies on the map having a civilian army, and the fallback is killing (exploding) the ACU according to the civilian desertion share condition.

Imagine if you did not get half the Factories or mexs from your allies because they DC'ed. The ACU early game is a small army onto itself.

This does remind me that the ACU has a large resource storage in the beginning of the game.

How about when a player disconnects, share their ACU + army and apply the share condition 5 minutes after the game started and 2 minutes after disconnect, or if the disconnected ACU dies early? The 5 minutes after start sharing allows use of the initial resources. The 2 minutes after disconnect lets the ACU explode due to natural game rules instead of using an ACU recall option, and gives time to move units away, use the ACU one last time, and get tech to stabilize vs enemy ACU.
I think this would be the most balanced way to avoid abuse while preserving benefits. In assassination fullshare, which is the matchmaker mode, it would be most balanced. In assassination partial/no share, it wouldn't be fair because of the unit loss, but those are custom games that can rehost.
Could add it as an option instead of the ACU recall setting. Maybe like: "ACU Share: Explode, If-safe Recall, Delayed Recall, Permanent". One of them can be set as ranked if it's considered worthy. Permanent share might as well unlock giving your ACU to allies too.

@apollodeathstar
Copy link
Contributor

apollodeathstar commented Mar 17, 2024

I think having it share for something like 2 mins after disconnect is a good idea, after which it should just recall or explode. that is enough time for the player to compensate for the missing teammate and adjust their play.

All I really care about doing is avoiding punishment for players in the early game who have a ally drop/dc and them suffering badly for it.

Also the 2 mins timer means players will be unlikely to waste mass upgrading the ACU. I think that is a good way to handle it.

Also I dont think the damage checks are necessary with a recall/explode after 2 mins system, as if the enemy was going to kill that ACU and a player DC's then I feel they are still just as likely to do so if not more so.

@lL1l1
Copy link
Contributor Author

lL1l1 commented Mar 21, 2024

We could also use the same value to 'enhance' the CTRL + K ability - if a unit did not receive damage the past 10 seconds then it can be destroyed without triggering the death weapon for example.

I think this is too overpowered for reclaiming volatile buildings, but it could be nice to prevent (possibly accidental) griefing with ACUs: if the ACU hasn't take damage recently and there are no enemies near the explosion range, it wouldn't detonate. After all, there's 0 benefit to the ACU dying without dealing damage because it leaves no wreckage.

@lL1l1 lL1l1 marked this pull request as draft March 26, 2024 10:36
@lL1l1
Copy link
Contributor Author

lL1l1 commented Mar 26, 2024

The ACU sharing mechanic is functional but it still needs proper UI with LOC tags and a better "received units from [player]" message that tells you the time remaining for delayed recall and the share condition after disconnect for delayed recall/permanent ACU share.

@Garanas Garanas force-pushed the deploy/fafdevelop branch from a3cba01 to fef28bb Compare March 31, 2024 20:00
@MrRowey MrRowey added area: sim Area that is affected by the Simulation of the Game area: ui Anything to do with the User Interface of the Game labels Apr 17, 2024
@clyfordv
Copy link
Contributor

Could:

  1. Put a timer on the transferred ACU, after which it becomes un-selectable.
  2. Similarly, start a timer when the receiving player gives the ACU an order, then make it un-selectable after that timer expires.
  3. As an addendum/combined with the above, give the ACU a move order back to its spawn point when it's transferred.

@lL1l1
Copy link
Contributor Author

lL1l1 commented May 1, 2024

Put a timer on the transferred ACU

I did that. I'm writing a new announcement box so that it can 2 lines of text but I'm open to better ideas to show how long that ACU has left. Maybe a new tab or a timer in the avatars tab.

after which it becomes un-selectable

Too out of the ordinary for how multiplayer skirmishes work, and it takes away control from the player for a key unit. It does make me wonder how the select/zoom to ACU hotkeys will work/break with multiple ACUs.

@Garanas Garanas changed the base branch from deploy/fafdevelop to develop June 1, 2024 19:35
@lL1l1 lL1l1 force-pushed the feature/recall-on-disconnect branch 2 times, most recently from 67715b0 to 3b8258d Compare June 5, 2024 22:54
lL1l1 added 12 commits December 24, 2024 19:29
Also fix transferring 0 units to self
Fixes GetBackUnits(), specifically for shared units in traitor mode, compared to the base develop branch.
Fixes KillArmyOnDelayedRecall function's LastTickDamaged check
Explode -> all acus die
Recall/Recall Delayed -> unsafe acus die
don't stack recalls on them
also don't count them as an ACU that keeps your army alive
- Don't put a `ratings` field into AIBrains when transferring units
- Give negative rated players some room
- Don't count AI rating when transferring
@lL1l1 lL1l1 force-pushed the feature/recall-on-disconnect branch from df0d289 to bea1ba0 Compare December 25, 2024 07:27
@lL1l1 lL1l1 marked this pull request as ready for review December 26, 2024 08:36
Copy link
Member

@Garanas Garanas left a comment

Choose a reason for hiding this comment

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

Some quick notes when purely reviewing the code.

local toBrain = GetArmyBrain(toArmy)
if not toBrain or toBrain:IsDefeated() or not units or table.empty(units) then
if not toBrain or (not noRestrictions and toBrain:IsDefeated()) or not units or table.empty(units) then
Copy link
Member

Choose a reason for hiding this comment

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

I think it is better to split this into different if statements. See for example:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about multiple lines like this?

if not toBrain or (toBrain:IsDefeated() and not noRestrictions)
    or table.empty(units)
then
    return
end

---@param damageType DamageType
OnDamage = function(self, instigator, amount, vector, damageType)
if self.CanTakeDamage and damageType ~= "TreeForce" and damageType ~= "TreeFire" then
self.LastTickDamaged = GetGameTick()
Copy link
Member

Choose a reason for hiding this comment

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

Won't this reference to LastTickDamaged be shared among all classes inheriting from this class? Because the value exists at the scope of the meta table. Not at the scope of the instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It won't because LastTickDamaged is a number and not a table. Tables would get shared at the meta table scope because the class system doesn't deep copy them, as we ran into when discussing enhancements.

lua/ui/game/announcement.lua Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: sim Area that is affected by the Simulation of the Game area: ui Anything to do with the User Interface of the Game
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants