FGA icon indicating copy to clipboard operation
FGA copied to clipboard

feat: Mighty / Color chain + NP type detection implementation

Open Vylantze opened this issue 4 months ago • 48 comments

  • fixes #1470
  • #1892

Download Link to Build APK


  • Added R.string.p_brave_chains_with_np_mighty for all languages
    • English uses the correct description
    • Japanese uses the correct katakana description
    • Korean used text description from Google AI
    • All other languages used english text
  • Current implementation uses the withNP implementation as the baseline and then modifies the final result of that to pull off a Mighty chain if possible (if not, it returns the normal result of withNP).
  • The intended implementation had been to also include NP, but it is stuck because NP card type cannot be detected.
    • Is it possible to make the user decide the NP card type by declaring it in the config...?
    • Currently using the card priority for each wave to determine the Brave chain desired, but it is not ideal.

EDIT: Intended Mighty chain implementation added with NP card detection (beta).

Screenshots

The config that was used to test is below. The map used is the current 90++ map in JP summer event 2025.

Overall Config Wave 1
FGA_Overall_Config FGA_Wave_1
Wave 2 Wave 3
FGA_Wave_2 FGA_Wave_3
Battle Config
FGA_Battle_Config
Sample run
FGO_Wave_2
FGO_Wave_3
P.S. Screenshots don't tally because they were taken from different runs, but rest assured that it is the same script running.

Event Box Farm.fga.json

Task List

Edit: Moved task list here for easier tracking

  • [x] Add handling for CardType.Unknown (particularly for NPs).
  • [x] Add tests for Mighty Chain to ensure correct behavior for all BraveChainEnum (None, WithNP, Always, Avoid).
  • [x] Add tests for Color Chain to ensure correct behavior.
    • [x] Card only handling (for all types Buster, Arts, Quick)
    • [x] NP handling (for all types Buster, Arts, Quick)
    • [x] Unknown card handling (for card and NP)
    • [x] Brave Chain handling for all BraveChainEnum (None, WithNP, Always, Avoid)
  • [x] Add tests for AvoidChain to ensure correct behavior for all BraveChainEnum (None, WithNP, Always, Avoid).
  • [x] Create a flowchart for how the system works, since it has grown immensely complicated with the integration of Brave Chain special handling into the Mighty / Color Chain systems. This is pretty important since I think it is very hard to review/test this pull request at the moment given how the expected behavior isn't properly documented.
  • [x] Add more integration tests for the CardChainPriorityHandler, so that the interaction and fallthroughs between each Chain type is properly tested.
  • [x] Add more integration tests for the AttackPriorityHandler, which determines the behavior between Brave Chains and Card Chains. This is not as crucial at the moment, because Brave Chains are treated as higher priority at all times for now (with the exception of None).
  • [x] Add automatic checks for NPs so that the bug where the system returns wrong result because of missing NPs/additional NPs from spam does not occur. (Best effort implementation. Might fail occasionally for spam NPs)
  • [x] Update documentation with the new system's details.

I intend to do the tasks from top down. The actual implementation changes will only change slightly (depending on what unexpected behaviors I find) while I perform these tasks. Most of the general cases should have a confirmed behavior already. The one most tested at the moment (particularly with actual testing in FGO) is WithNP, because that covers majority of the use cases.

Flowchart

Show FGA flowchart

Vylantze avatar Aug 23 '25 21:08 Vylantze

Build 737

Download the latest APK for testing here

[!NOTE] You need a GitHub account to download the APK.

This URL is valid as long as the artifact has not expired yet.

github-actions[bot] avatar Aug 23 '25 21:08 github-actions[bot]

As of 553985c update, I have added a beta NP type checker to the system. This will mean that the NP type is automatically detected, which means that priority for each wave won't be needed anymore.

Flaws / Potential bugs:

  • Does not work properly with spam
  • If the command line has an NP, it will assume that the NP is valid and won't do any other check on it. If your NP for some reason does not actually trigger, it results in a less optimal Mighty Chain (or even a Mighty Chain failure).
  • The card type check can probably fail with some Servants/regions. So far, I have only tested JP and with only a handful of Servants.

Other than that, it works!!

New screenshots (collapsed) image image image image image image image

Vylantze avatar Aug 28 '25 17:08 Vylantze

Tested on the Dubai bleached earth quest using np from douman and face cards from bb and haven't had any issues after the np checker update

vybze avatar Aug 30 '25 08:08 vybze

I intend to make a pretty major change to the way that Mighty Chains are determined, instead of tacking it onto the Brave Chain dropdown. I will drop the With NP (Mighty) option from the Brave Chain dropdown after it is implemented.

This will accomplish the following:

  • Divorce the Brave Chain from the Mighty Chain. Allows for Brave Chains without Mighty and vice versa.
  • Add ability to specify usage of specific colored chains, which has now become possible due to the new NP checker (beta).
  • Actually fixes this issue, because it will enable the active prevention of the colored chains and Mighty chains.
  • Minimizes the changes to Brave Chain functions, allowing people to continue using Brave Chain even without turning this on. This allows for any potential bugs to not actually break any current versions of FGA and can be more easily reverted than adding a new enum to Brave Chain for future colored chains.

P.S. If anyone has any recommendation for the color codes of Mighty and Avoid, it would be appreciated. Currently just using an unused palette from the already available colors.

Preview of what it will look like
image image
Chain Priority disabled
image

Vylantze avatar Aug 30 '25 18:08 Vylantze

I intend to make a pretty major change to the way that Mighty Chains are determined, instead of tacking it onto the Brave Chain dropdown. I will drop the With NP (Mighty) option from the Brave Chain dropdown after it is implemented.

This will accomplish the following:

  • Divorce the Brave Chain from the Mighty Chain. Allows for Brave Chains without Mighty and vice versa.
  • Add ability to specify usage of specific colored chains, which has now become possible due to the new NP checker (beta).
  • Actually fixes this issue, because it will enable the active prevention of the colored chains and Mighty chains.
  • Minimizes the changes to Brave Chain functions, allowing people to continue using Brave Chain even without turning this on. This allows for any potential bugs to not actually break any current versions of FGA and can be more easily reverted than adding a new enum to Brave Chain for future colored chains.

P.S. If anyone has any recommendation for the color codes of Mighty and Avoid, it would be appreciated. Currently just using an unused palette from the already available colors.

Preview of what it will look like image image Chain Priority disabled image

Since you're adding a new priority I'm curious about how this card priority would interact with the other priorities. We've seen that having too many priorities active at the same time can cause issues, for color maybe red Mighty and gray for avoid

vybze avatar Aug 30 '25 18:08 vybze

Since you're adding a new priority I'm curious about how this card priority would interact with the other priorities. We've seen that having too many priorities active at the same time can cause issues, for color maybe red Mighty and gray for avoid

I made an update to the colors to more clearly visualize what happens to the priorities for those past Avoid. Gray looks pretty good for Avoid, but I would rather not red for Mighty since that does clash with Buster.

image

As for the card priority handling... well, that is an excellent question and something I had to think real hard on how to handle.

First off, I intend to use a completely different implementation from the current one for Chain Priority. However, this implementation will only be used if you toggle Chain Priority on. Otherwise, it will default to the current implementation.

So, based on the screenshot above, I'm trying to get the user to intuitively understand how it works. Take a look at it one more time and think about how you think it will work based on what you see, then read on.

First, Chain Priority will determine what types of chains will be checked. Anything behind Avoid will basically be actively avoided. So, in this example, Mighty and Buster chains will attempt to be checked, but it will not do Arts and Quick chains unless there is no choice. So, what will happen to those in front? Let's have an example to illustrate. Let's say we have a BBBQA. This is the only types of situations where the priority matters based on which order you put it. So, in this scenario, it will choose the Mighty chain first over Buster chain.

As for the interaction of Chain Priority with other Priorities, well... the final conclusion is that I'll most likely make it another setting in a different pull request. It does severely complicate things, but it is much more explicit to the user than the current method of implementation (where no one really knows which priority takes precedence over another).

There are currently 4 priorities, which actually take precedence in the following order.

  1. Brave Chain Priority
  2. Chain Priority (newly added)
  3. Servant Priority
  4. Card Priority

Changing this order in any way does require quite a big overhaul, and it is quite hard to do that without risk of users suddenly experiencing very different behavior. Technically, Chain Priority can be put above Brave Chain, but I suspect most users have a single DPS and that single DPS is almost always having an NP, so having Brave Chain be prioritized over a specific chain type is better. Of course, if it is something like Olga Marie sk1 where you get everything, then Chain Priority will work properly within Brave Chain. That is the most useful case for the majority of use cases.

Thanks for raising it by the way. I did already have an intention to do the explicit Priority sort, but penning it all down made it a lot clearer for myself as well. I probably subconsciously planned to do it as part of this pull request at the start of writing this comment, but finally decided to not do so and put a different pull request for that later.

TLDR: When Chain Priority is on, the Brave Chain behavior will be affected in the following manner. When it is off, it will work exactly like it does now.

Brave Chain Description
Don't care Chain Priority will take precedence. It will ignore checking for Brave chains and just take the first cards that fulfil the first fulfillable Chain Priority condition.
With NP Brave Chain will be more important. It will ignore Chain Priority if there is a Brave Chain available with the NP, and will only do Chain Priority if there is a flexibility to do so within the cards provided (e.g. Olga Marie/Ciel).
With NP (Mighty) Will be removed once Chain Priority is added. Users that do not change out of this by the time it is removed will have their behavior defaulted to Don't care.
Avoid Brave Chain avoidance will be more important. It will ignore Chain Priority if the requested chain can only be done with a Brave Chain.

Vylantze avatar Aug 30 '25 21:08 Vylantze

It is not like this is needed to be added here, but if this ever get merged could you also write in the future on how it works as a guide

https://github.com/Fate-Grand-Automata/FGA/blob/master/wiki/scripts/Battle.md#card-priority-optional

ArthurKun21 avatar Aug 31 '25 01:08 ArthurKun21

Well, the deed is done. The heavy lifting is over.

image image image 2025-09-03_09 31 15_studio64_O8dLjbZhNb

As per the screenshots above, this is the final result.

How does it work?

A new option has been added to the Brave Chains dropdown, called Always (also translated in all languages via google translate). This option only appears when Chain Priority is toggled on. The behavior of Brave Chains and Chain Priority will be as such:

Brave Chain Description
Don't care (Same as above) Chain Priority will take precedence. It will ignore checking for Brave chains and just take the first cards that fulfil the first fulfillable Chain Priority condition. It may sometimes accidentally Brave Chain due to how the cards line up.
With NP (Same as above) Brave Chain will be more important, but only when an NP is the first card. It will ignore Chain Priority if there is a Brave Chain available with the NP, and will only do Chain Priority if there is a flexibility to do so within the cards provided (e.g. Olga Marie/Ciel).
Always (new) Brave Chain will be of utmost importance. It will always Brave Chain if there is one available, no matter which character used, ignoring everything else.
Avoid (Same as above) Brave Chain avoidance will be more important. It will ignore Chain Priority if the requested chain can only be done with a Brave Chain.
Chain Priority Description
Mighty / Buster / Arts / Quick Attempts at the respective Chain will be made. If it fails to make one, it will fall through to the next item in the list, unless the item is Avoid.
Avoid If this is the first item in the Chain Priority, all Chains will be avoided. It will only avoid Mighty / Buster / Arts / Quick chains, unless Avoid is also chosen for Brave Chains, after which it will also avoid Brave Chains. If it is NOT the first item on the list, it will be the cut off for Chain checking, after which it will fall to the default order (which is Servant Priority + Card Priority, normally).

What this means is that in the default order of Mighty -> Buster -> Arts -> Quick -> Avoid, it will always try to make a Mighty Chain first with the cards available (including NP where applicable). Then, if one cannot be made, it will attempt to make a Buster Chain. Failing that, it will attempt to make an Arts Chain. If this succeeds, it will return the newly formed Arts Chain and ignore checking for Quick.

An unexpected advantage I've found with this system is that you can now arrange the card priority in order of QAB and still get to do your Buster chains, while your Mighty chains will properly follow QAB order.

Sample (collapsed) image

My Ciel fighting Flare Marie does a NPBB for the first turn, then follows up with an NPXX, where X is influenced by whatever cards that appear. Normally, this results in either a Brave Chain that ignores the setup, or if there are not enough Ciel cards, it usually results in a Mighty Chain with BQA. Then, sometimes on a third turn (because sometimes my defense down misses), it executes a proper QAB Mighty Chain.

Flaws / Potential Problems

As mentioned above, there are several problems with this system at the moment.

  1. It is unable to work with Spam in the intended manner. To be more specific, it fails when Spam clicks NPs automatically, since the NPs clicked by Spam are not given to the system to process. If NPs are never set to Spam, the system works as intended. *This can be potentially fixed in a separate pull request to make NPs detectable while spamming.
  2. NPs being missed in command. If the command line has an NP queued up, it will be fed into the system. Thus, if for any reason the NP fails to be executed, the system will assume that the NP is still queued up and behave as such. *This can be potentially fixed in a separate pull request to make NPs detectable, thus knowing when an NP is missed in the command line.
  3. The NP type detection system (beta) that was newly added is essential to making the chain priority work in the use cases that most players expect. However, since it is new, there could be potential problems with its detection failing. More users / more extensive testing is needed, since this cannot be tested with unit tests. In particular, since I developed this for the JP side, I am worried about the other languages (most notably NA).
  4. More unit tests. The system has had a lot of new tests added to ensure it works as intended. However, I have not finished adding tests for everything, so that's something I'll keep doing, and people can also suggest specific scenarios where they think the system might be flimsy for me to add as test cases.

Otherwise, the pull request is ready for review and merge. All I'll be doing now is doing more of 4, and add even more tests for every subsystem (of which there are 6 major subsystems, 1 utility subsystem). I also would like to finish up the integration tests. But, I decided to push it now so that actual people can get to testing too, since I am only one person and I have already added tests for every edge case that has crossed my mind while implementing/doing my own tests.

On a side note, yesterday night I ran the system the whole night and by morning, I still saw it still running and Ciel-senpai still murdering Flare Marie. So, at least, it passes a sustainability check and the system works for long hours.

Vylantze avatar Sep 03 '25 02:09 Vylantze

Summary of things done

  • Merged master into the current branch.
  • Made changes to the implementation based on test cases added, to ensure that the behavior is the most expected one. (Most of it involving AvoidBraveChain)
    • Example: Changed handling of AvoidBraveChain to permit a Mighty / Color Chain to persist if all cards are from the same Servant, under the condition that all available cards belong to that same Servant. Previously, the Mighty / Color Chain is rejected if it fulfils any form of Brave Chain.
  • Implemented AvoidBraveChain special handling for all Mighty / Color Chains. Now, the system will try its best to procure a Chain if it is possible to substitute the original selection for another card with the same traits. Not sure if the performance will be affected noticeably or not until I run a few endurance tests tonight.

Task List

Moved to top.

Vylantze avatar Sep 04 '25 19:09 Vylantze

sample

add handler for unknowns

ArthurKun21 avatar Sep 05 '25 08:09 ArthurKun21

Is it not possible to specify which card color to start or end the Mighty Chain with?

vybze avatar Sep 05 '25 11:09 vybze

Is it not possible to specify which card color to start or end the Mighty Chain with?

The Mighty Chain order is currently determined by the Card color priority (HIGH to LOW). I can add a description somewhere to indicate this better in the app itself, or consider adding a separate one exclusively for Mighty Chains at a later time.

Vylantze avatar Sep 05 '25 11:09 Vylantze

we can just add it at a later time as the number of lines for the PR is getting a bit bigger.

Might take time for reconman to check on this

ArthurKun21 avatar Sep 05 '25 11:09 ArthurKun21

By the way, what is the current behavior when the NP cannot be detected? I checked my code and it properly refers to it as Unknown if it fails to match it. The Handlers do not explicitly handle Unknown since it is filtered out near the start of the highest handler, but since Unknown NPs should not be castable, it shouldn't break the system... I hope...

Vylantze avatar Sep 05 '25 11:09 Vylantze

iirc, currently there is no detection if NP was available or not.

it always click all of the before cards, NPs and then after cards.

The cards would always have at least 3 command cards in case the NPs weren't available

Since there is no detection in NP, if the NP wasn't available or like servant is stunned/np locked/etc. It has a auto close to it

https://github.com/Fate-Grand-Automata/FGA/blob/b5819d76563dfdab19178a5ef39f9235fd2d064d/scripts/src/main/java/io/github/fate_grand_automata/scripts/modules/Card.kt#L84-L87

https://github.com/Fate-Grand-Automata/FGA/blob/b5819d76563dfdab19178a5ef39f9235fd2d064d/scripts/src/main/java/io/github/fate_grand_automata/scripts/modules/Caster.kt#L181-L186

ArthurKun21 avatar Sep 05 '25 11:09 ArthurKun21

iirc, currently there is no detection if NP was available or not.

it always click all of the before cards, NPs and then after cards.

The cards would always have at least 3 command cards in case the NPs weren't available

Since there is no detection in NP, if the NP wasn't available or like servant is stunned/np locked/etc. It has a auto close to it

https://github.com/Fate-Grand-Automata/FGA/blob/b5819d76563dfdab19178a5ef39f9235fd2d064d/scripts/src/main/java/io/github/fate_grand_automata/scripts/modules/Card.kt#L84-L87

https://github.com/Fate-Grand-Automata/FGA/blob/b5819d76563dfdab19178a5ef39f9235fd2d064d/scripts/src/main/java/io/github/fate_grand_automata/scripts/modules/Caster.kt#L181-L186

Yeah, that is one of the issues with the syatem. I was just worried that the actual system breaks or causes the app to crash because I didn't explicitly deal with the Unknown npType.

Vylantze avatar Sep 05 '25 13:09 Vylantze

oh the unknown was mostly for the chain color on whether mighty or other colored chains so you can have a fallback just in case it fails to detect or such cases from above with the support servants

484868496-27450714-e6a7-41a9-9e61-9bc5f6b1c79f

ArthurKun21 avatar Sep 05 '25 13:09 ArthurKun21

https://github.com/Vylantze/FGA/blob/basic-mighty-chain/scripts/src/main/java/io/github/fate_grand_automata/scripts/modules/attack/AttackPriorityHandler.kt#L67-L71

The system does already deal with Unknown cards in its own way. Do you want me to add a note for this?

Also I realise now that this is risky because there is a non-zero chance that an invalid set of cards is returned and causes FGA to stop, so I'll rework this a bit.

But, the idea is that the system will completely skip handling the cards if the card types are unknown, so it will just be whatever default order the cards come in.

Edit: I went to find places to try to add CardType.Unknown checks, but it turns out that I did already handle them without realizing it, due to the way the checks are handled. I'll add some test cases for CardType.Unknown though, just in case.

Vylantze avatar Sep 05 '25 13:09 Vylantze

Also I forgot to say about this

https://github.com/Calvin-LL/Reorderable

So we can remove the recycler view on card priority and replace it with compose for better integration

ArthurKun21 avatar Sep 07 '25 02:09 ArthurKun21

Also I forgot to say about this

https://github.com/Calvin-LL/Reorderable

So we can remove the recycler view on card priority and replace it with compose for better integration

This looks pretty good, but looks like it should be a pull request by itself. Also because I don't know kotlin well enough to integrate this with confidence.

Vylantze avatar Sep 07 '25 16:09 Vylantze

I see then I'll just wait for your PR to be merged before doing this so you don't have to rebase

ArthurKun21 avatar Sep 07 '25 16:09 ArthurKun21

It was easier than I expected, so I am adding np detection to the system. As usual, it will only be available when Use Chain Priority is turned on, but if it works well, it will be easy to use it for normal mode as well.

Raw: https://streamable.com/dvry9q

https://github.com/user-attachments/assets/b20a8169-e72f-44cf-b0b8-36a3e089f0fa

Vylantze avatar Sep 07 '25 19:09 Vylantze

Added a new ChainTypeEnum that serves as the cutoff for priority handling instead of the older build where it relied on Avoid instead.

2025-09-08_22 47 27_studio64_Zdc3xBw97l 2025-09-08_22 47 39_studio64_7ebfVGIKzm 2025-09-08_22 47 55_studio64_T2oJ1BPzLr

Vylantze avatar Sep 08 '25 14:09 Vylantze

This is a bit unreadable.

image

ArthurKun21 avatar Sep 08 '25 15:09 ArthurKun21

What would be better color scheme for the background + text for disabled sections then?

Vylantze avatar Sep 08 '25 18:09 Vylantze

What would be better color scheme for the background + text for disabled sections then?

Make the text black, you would need to change the color for mighty chain tho

vybze avatar Sep 08 '25 19:09 vybze

I was worried that if I make it black, it won't look 'disabled' enough.

image

How is this? No matter what color I try, it just doesn't look quite right, since I want it to look something like this.

2025-09-09_04 03 33_chrome_Dpb2S5JwxM

EDIT: Never mind. It was just a bug. Fixed it and now it looks just the way I want.

image

Is this still too hard to read?

Vylantze avatar Sep 08 '25 20:09 Vylantze

I was worried that if I make it black, it won't look 'disabled' enough.

image How is this? No matter what color I try, it just doesn't look quite right, since I want it to look something like this. 2025-09-09_04 03 33_chrome_Dpb2S5JwxM EDIT: Never mind. It was just a bug. Fixed it and now it looks just the way I want. image Is this still too hard to read?

On mobile it's a little better but on PC I have to lean closer to the monitor to read it. I recommend you try a stronger color for the text if you don't want to use black

vybze avatar Sep 08 '25 21:09 vybze

Is this still too hard to read?

much better than before.

lmao this is why I like coding with compose it is just easier to change the color

MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.38f)

ArthurKun21 avatar Sep 09 '25 01:09 ArthurKun21

image

I tried a darker gray. Hopefully, this is better.

much better than before.

lmao this is why I like coding with compose it is just easier to change the color

MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.38f)

Looking through the code, I do see some parts of the code uses this for the general UI color. I have been editing the colors directly on the xml because that's just what the section I'm editing does, so I'm following it. When you are doing the refactor for it, you can probably make them all use Compose properly.

Vylantze avatar Sep 09 '25 02:09 Vylantze