simimpact / srsim Goto Github PK

Some logic (such as bounce attacks) will execute "Retarget" behavior in game. This is used to filter a list of targets down in 3 ways:

1. Removes all targets that are at 0 HP from the list (optionally including targets in limbo)
2. Can be provided an optional filter function
3. Can set a max output (randomly selecting from the options)

This is important when factoring in #43 which would result in 0 HP enemies being included in engine.Characters() and engine.Enemies() in certain scenarios

For sim Retarget (maybe multiple versions/overloads) input params (probably want an info.Retarget input struct):

1. list of targets to filter
2. filter function
3. Max (limit number of output targets, randomly selecting from valid candidates)
4. flag for if 0 HP targets should be included

Unknown/unclear from TC: When allowing 0 HP targets in return w/ a Max set, the random selection will preference living targets

The game has an AddRegardAsAttackType function which allows modifiers to append additional AttackType tags to existing attacks. This enables attacks to be treated as multiple attack types. This is currently used in the Champion's Dinner: Cat's Cradle blessing which has a description of "Characters' Ultimates are considered as a follow-up attack. Follow-up attacks deal increased DMG."

In the StatusConfig of the datamine, only specific statuses/modifiers can be dispelled rather than all of them. Need to add a CanDispel boolean to our modifiers to mimic this behavior.

Currently sim only supports a single wave of enemies. The SimConfig schema + sim core loop needs to be expanded to support multiple wave combat.

All implementations should also be checked for adequate multi-wave support (combat start buffs apply on wave 1 and not all waves).

Also need to consider how TotalAV will be calculated in the context of waves. In MoC, when you finish a wave mid-cycle, it resets to "beginning of cycle" on the next wave. This means just summing up total AV across all waves is not accurate to measure number of cycles (instead is number of cycles per wave rather than TotalAV / 100). Might be okay for sim data though?

In the case of seele A6, it is neccessary to add a condition that if :

• current turn is seele's, and
• she's on her resurgence turn

That the advance forward be delayed to the TurnEnd.
In order to make this comparison, we need to add engine access to who the current turn belongs to.
Might want to resolve this issue along with #187 as both problems seem similar.

Cli is not running on windows without also running -k
The cli should apply the same fix as genshinsim/gcsim#1417

In 1.2 they added a new property which limits the amount of HP that is healable. This is done as a new property that is only used in the heal formula, reducing the max cap from 100% to 100% - DirtyHPRatio

The invincibility part of Arlans A6 has not been implemented yet. Once the core supports invincibility to everything except dots, this will be fixed.

When a break damage hit is performed, the break effect multiplier should only be used in the BaseDamage. In the current implementation, it is multiplying against the BaseDamage and DamageValue which is not correct.

When applying a modifier to yourself with a Chance, should be a fixed chance and not use the resistance formula. Most/all cases of this should be calling engine.Rand() instead, but should still have this logic as a safety net for the case of odd behavior

Enemy Info should include the enemy rank/status. Some logic (such as break effects) change depending on if the enemy is elite or not.

Common character behavior is to add a buff to all allies. In the current state this has to be done in two parts during the Create:

1. looping through the list of all characters in engine atm
2. subscribing to the CharacterAdded event so you get notified when a new one is added (IE say character adding buff was 1st in turn order)

Adding an AllCharactersAdded event which sends after all character creates occur will simplify these cases.

Similar reasoning for adding the AllEnemiesAdded variant

On the implementation of the Light Cone Post-Op Conversation, it is found that the DM uses the listener OnBeforeDealHeal and then check if the source of the heal is an Ultimate.
Current implementation of OnBeforeDealHeal does not have AttackType prop to do a check, thus need to do a non-faithful implementation using OnBeforeAction and OnAfterAction (need tc confirmation if both are equivalent).
If future similar instance of this issue exist, might need to add the prop into the event listener.

• Add AttackType prop to OnBeforeDealHeal Listener

Some characters have modifiers which gives them guaranteed debuff resistance. This is done using a ResistDebuff behavior flag which will cause debuffs to always be resisted when applied. Need to update resist logic to check for and use this flag

As part of this change, should make sure we have the necessary modifier-based listeners. OnResistDebuff and OnResistBuff should be added

The Count and MaxCount field of modifiers is typed as float64 but should be int. Exisisting implementations using modifiers should be fixed if they relied on them being float64.

https://github.com/simimpact/srsim/blob/main/pkg/engine/modifier/modifier.go#L19

Currently the only supported method for declaring relic stats is by defining the main and sub stats of each relic attached to the character. Want to support alternative methods to enable other approaches (will also need to expose some of these alternative approaches on UI)

With 1.2, the following now use the equivalent OnSnapshotCreate and can modify phase1 damage (dots + freeze + entangle). Implementations need to be updated to reflect this:

LightCones:

• Shattered Home: https://starrailstation.com/en/lightcone/20009
• Loop: https://starrailstation.com/en/lightcone/20011
• Good Nightand Sleep Well (OnSnapshotCreate was removed): https://starrailstation.com/en/lightcone/21001
• A Secret Vow: https://starrailstation.com/en/lightcone/21012
• In the Name of the World: https://starrailstation.com/en/lightcone/23004
• Incessant Rain: https://starrailstation.com/en/lightcone/23007
• Fermata (OnSnapshotCreate was removed): https://starrailstation.com/en/lightcone/21022
• Woof! Walk Time! (OnSnapshotCreate was removed): https://starrailstation.com/en/lightcone/21026
• Cruising in the Stellar Sea: https://starrailstation.com/en/lightcone/24001

Relics:

• Genius of Brilliant Stars: https://starrailstation.com/en/relics/108
• Wastelander of Banditry Desert: https://starrailstation.com/en/relics/112

Character Mechanics:

• Dan Heng E1
• Himeko E2
• Hook E6
• Pela A2
• Physical MC E4
• Seele E1
• Serval E6
• Silverwolf E6
• Welt A6
• Yanqing Technique

In current sim implementation, when a target reaches 0 HP we immediately run a LimboWaitHeal. If no subscribers cancel it, we then emit a TargetDeath immediately removing this target from the battlefield

Based on current TC, this is not the correct behavior. Instead, when a target reaches 0 HP we should still put it in LimboWaitHeal state, but then defer its death to the end of the current combat phase. This means that by default, any attacks will continue to occur on 0 HP targets and the target will continue to exist on the battlefield (will be contained in engine.Enemies() or engine.Characters()).

The new flow should roughly look as follows:

1. attribute keeps track of a LastAttacker for each target
2. when a target reaches 0 HP, attribute emits a LimboWaitHeal event
3. attribute marks this target as InLimbo if LimboWaitHeal was canceled (some subscriber intends to revive)
4. sim performs a "death check" at the following locations. Any targets that have HP <= 0 and not InLimbo will be "killed" (engine removes them from battlefield, turn order, and emits a TargetDeath event)
   • In phase1 after ModifierPhase1
   • After ActionEnd
   • After InsertEnd
   • in phase2 after ModifierPhase2/at start of endTurn

This solution also solves/closes #41

Like attacks, heals in combat should do nothing if target is dead (this is why revives must call SetHP directly)

Some implementation logic (such as Seele's talent) needs to know who the current action is owned by. Depending on where the checks occur, this information may not be readily available. Need to store this information in simulation and provide a helper in the engine interface to simplify implementations

Documented observations:

• Silver Wolf has unique toughness mechanic that triggers her own break effect regardless of enemy weakness. May need to account for supply custom break type to event handler.

Add a utility method to get modifiers from source rather than getting all modifiers. Useful when wanting to get the instance of a modifier that is using ReplaceBySource stacking behavior

• E4 adding all the logic
• Bounce is Retarget
• Ulti is Retarget

In the logs, it may be hard to tell from a single event why it happened. Adding more keys can help to assign ownership to these events and improve the traceability of our data.

non-exhaustive list of keys to add:

• key.Attack
• key.Heal
• key.Insert
• key.Reason (a generic key where all key types are usable/just string?)

Places where we would want to pass in a Reason:

• RemoveModifier + RemoveModifierFromSource (not needed)
• RemoveShield (not needed)
• ExtendModifierDuration + ExtendModifierCount (not needed)
• SetHP
• ModifyHPByRatio
• ModifyHPByAmount
• ModifyStance
• ModifyEnergy
• ModifyEnergyFixed
• ModifySP
• SetGuage
• ModifyGuageNormalized
• ModifyGaugeAV
• SetCurrentGaugeCost
• ModifyCurrentGaugeCost
• DispelStatus
• Mutable Events (need to abstract fields to limit mutability)

CI should fail if the generated code is not fully up to date.

Random example found online: https://github.com/tendermint/tendermint/blob/main/.github/workflows/check-generated.yml

In game Arlan's talent is only added and counts as a BUFF when his HPRatio < 1.0. In the current sim state, the BUFF is always active/applied to Arlan even though it gives a +0 DMG% increase on the 1.0 case.

Two possible solutions:

1. split into two modifiers: 1) logic modifier that adds/removes buff depending on if at 100% HP
2. add support for conditional status types #17

Plan to setup CI linting to enforce better code quality. Will start with minimal set of rules and gradually increase as necessary. Creating an issue to track what settings we want to move towards and what changes are required to make the codebase compliant.

In the even the logic.Eval attempts to perform an action (attack or skill, not ult) that is not currently possible, emit an ActionFailed event before defaulting to the defined fallback behavior. This will help for traceability + can provide warnings for such cases in the UI (warning that sim may not be functioning as user intended)

Method to get a count of stacks for a given modifier. In the case of duplicates (Multiple, ReplaceBySource stacking cases), sum across duplicates? Optional filter function/logic (IE: only count modifiers from source target?)

Currently the TurnStart event emits when StartTurn is called in the turn package. While functional, it shouldn't be up to the turn package to dictate when TurnStart actually is. Want to push this emission ownership to the simulation run loop (which is also where EndTurn ownership lives).

Current priority implementation puts the ult & action insert priority at different values. This means for cases such as Seele's resurgence will always go after any ult in the queue:

this does not match what happens in game. In game, the priorities should match (Seele's resurgence has sime priority as ults). This generally makes sense given both ult and action inserts are both "actions". New priorities should be:

CharInsertAction  InsertPriority = 500
EnemyInsertAction InsertPriority = 1000

As part of this change, can also add more insert priorities for other categories: https://github.com/Dimbreath/StarRailData/blob/1ab86f99405026f6c9b1be98661a584e1a38a0df/Config/GlobalConfig/PriorityConfig.json#L26-L58

When InsertAbility is called, a list of targets should be specified to match the game behavior. If all targets are dead or in limbo, the insert should be canceled.

In the current sim implementation, ReplaceBySource and Replace stacking behaviors will trigger a ModifierRemoved event and call the OnRemove listener when being replaced (IE: another application is occurring). This results in any removal behavior being triggered:

This does not match the game behavior. In-game, these replace stacking/refresh scenarios do not cause a removal to be triggered. These emitRemove calls should be removed from these stacking behaviors

Right now modifiers manage shields. In the event a shield is destroyed by damage, the modifier will persist in the current state. Need an OnShieldRemove listener to allow modifier implementations to self remove the modifier when the shield is destroyed.

Following logic in Pela implementation should be changed to match game:

• E1: Pela gains energy when any ally kills an enemy (should change to TargetDeath event subscription)
• E6: Only living targets get hit (game uses Retarget for E6: #44)
• Talent: Only living targets that were hit count towards the Debuff Counter (game uses Retarget: #44)
• (from #19): A4 OnBeforeDying case

When a TargetDeath event is emitted, the simulation must update the current sim state to remove the killed target from the targets list + turn order. This should always happen first. In the event that something subscribes to the TargetDeath event, this target should no longer be found in the battlefield (engine.Enemies(), `engine.Characters(), etc). In the current state of the sim, this is not guaranteed.

To guarantee this, we have two options:

1. The engine must own emitting the TargetDeath event (and do the necessary maintenance prior to emission)
2. The TargetDeath event must use a PriorityEventHandler where this maintenance logic has highest priority

Today, the TargetDeath event is owned by attribute.

If multiple InsertAbility calls are performed for the same key, only the first call should be used and enqueued.

For cases like Mutual Demise and Dan Heng A2, property is conditionally applied based on some threshold. To mimic the game's implementation without creating a 2nd modifier, would need to conditionally change the status type of a modifier within its implementation logic (only be considered a BUFF iff HP < 80%)

Pending TC testing before we can commit to an implementation. Needs to be implemented for the following abilities (non-exhaustive):

• Pela A4

Enemies such as SU Gepard summon additional enemies to help fight. This is something not easy to sim due to the data collection issues this poses (how do you now aggregate a dynamic list of targets across all iterations).

An alternative approach can be to instead give enemies the ability to "reset" other enemies on the field. This can be treated similar to a revive, but acts more like inserting a new target fresh in the order (but reusing an existing TargetID).

Current implementation of use snapshot applies to *BeingHit* and *DealHeal. From further TC, snapshot cases only apply to when the target is the actor (performing the attack or heal). In the case of receiving attack/heal, should always apply regardless of if it is a snapshot case or not.

currently trace ids are equal to the trace ids used in the game/dm. While this is simple to map to game data, this is hard to use and user define (each character has a different set of ids). These ids should be updated to be a normalized set that are the same across all characters (this can be done by truncating the char id from the trace id).

want to add selfupdate to simplify keeping folks on the latest binary. This requires tagged versioned releases to be setup and enabled https://github.com/creativeprojects/go-selfupdate

Currently combat stores the key of the attack call that started the attack to reuse ad part of the AttackEnd event. In the case of multiple hits with different ids, the AttackEnd key may not make much sense (only the first-used key will have an attack start and end). May want to remove key from attack events or think of a new key approach that guarantees more consist key behavior that is easier to trace.

Currently info.Stats only shows the final state/status of the snapshot. It is missing any insights into why it has been built this way. It should have details breaking down the following:

• BaseStats (info.Attributes?) that are used for this snapshot
• Snapshot of each modifier containing lists of what it provides (IE: ATKPercent: 0.1, weakness: ICE, etc)
• Immutable audit log of any changes that were made directly to this stats snapshot (capture AddProperty calls into a list)

In addition to the above, want to display more stats in the final stats list. At minimum should match same stats as game UI

Current action struct is generic and will have either Target and/or TargetEvaluator set which makes it hard to determine how it will function. Action should change to an interface and have potentially multiple implementations

As part of 1.2 updates, they moved the Qingque EndAttack to occur before card state changes/immediately after damage. Need to move the EndAttack call to reflect the new state of the game

Needed for Past and Future and Bronya LC. Should be a function that returns an []key.TargetID which is a copy of the current turn order. Implementations can then filter this down to find their desired target based on the current turn order.

Currently three light cones in this game use this stacking behavior:

• Chorus
• Day One of My New Life
• Carve the Moon

This stacking behavior works as follows:

1. Adds to the target using the ReplaceBySource stacking behavior
2. When EvalModifier is called, modifiers with this behavior are deduplicated based off key.Modifier

What this means is:

• If you have multiple sources applying the same modifier: both modifier instances exist but only one is effective at any given time (must be the last applied modifier instance that is active). This is identical to the Replace stacking behavior where the last applied becomes the only active instance.
• If one modifier instance is removed, the other becomes the new effective modifier when evaluating (this is different from the Replace stacking behavior where the old instances are permanently removed).

On the implementation of the Light Cone Quid Pro Quo, it is found that one of the condition of this light cone's effect activation is if an ally other than the LC's holder have <50% energy. This calculation can only be made if a character's current energy and their max energy are known.
But currently, the only way to get a character's max energy is by using Engine().Stats() which is computationally expensive to use.
Proposal to add in .MaxEnergy() method to Engine().CharacterInfo()'s first output (Info.Character struct specifically).

• Add .MaxEnergy() or .EnergyRatio() method to Info.Character struct used as Engine().CharacterInfo()'s output