Hanmac's Refactoring Thread
Post MTG Forge Related Programming Questions Here
Moderators: timmermac, Blacksmith, KrazyTheFox, Agetian, friarsol, CCGHQ Admins
Hanmac's Refactoring Thread
by Hanmac » 20 Dec 2016, 18:33
in this thread i want to post and collect the things i am doing refactoring within forge.
i want to make an extra thread for that to keep the bug threads free from it and to see your opinions about my changes before i do them.
First Topic:
"Splice (onto) <Arcane>"
Now:
Refactored:
Missing:
i want to make an extra thread for that to keep the bug threads free from it and to see your opinions about my changes before i do them.
First Topic:
"Splice (onto) <Arcane>"
Now:
- currently it does show a massive list there you try to select which of the many combinations you want.
- also there is currently no way to select the order, but the rules explicit says that you choose an order (even if it might not have a big effect from the spells)
Refactored:
- i did remove it from GameActionUtil and make a new function in AbilityUtils#addSpliceEffects(SpellAbility) which does return the cloned and modified SpellAbility object. (cloned to prevent it from changing the main object)
- inside there is a call getController().chooseCardsForSplice where the player can select the cards to splice similar to the Effects of a Charm Effect. (Gui().many(0,cards.size()))
Missing:
- The AI currently does not know what do to with that. should that be done in PlayerControllerAi or should that go to AiController?
- my Plan is that the AI does try to find the best combination (irrelevant order) of Splice Cards it has the mana to pay for.
Re: Hanmac's Refactoring Thread
by Hanmac » 21 Dec 2016, 16:17
My current sollution for the AI choices for the splice cards:
First order them descending by cmc (thinking that better cards cost more)
Then try to add cards with checking again if AI does still want to play that combined spell and can pay the cost.
Not tested yet but it should work.
The only problem with that is that AI will probably prefer to pay the more costing card instead of splicing into a cheaper spell.
First order them descending by cmc (thinking that better cards cost more)
Then try to add cards with checking again if AI does still want to play that combined spell and can pay the cost.
Not tested yet but it should work.
The only problem with that is that AI will probably prefer to pay the more costing card instead of splicing into a cheaper spell.
Re: Hanmac's Refactoring Thread
by Hanmac » 21 Dec 2016, 21:28
because the Splice thing was finished faster than i wanted, we might discuss the next topic. (even if the AI part might not be perfect yet)
next thing is something for AI.
while testing how CounterMoveAi works, i tested Evolve Trigger.
Specially Simic Fluxmage and Cytoplast Root-Kin because of their different MoveCounter Effects.
while testing i noticed that the AI currently has no logic how to stack Abilities that are put on the stack the same time.
in this example the AI should prefer to resolve Evolve Trigger first before PutCounter(All) Effects, so the creature can benefit from that.
another example would that the AI should prefer Draw Effects before Discard Effects (even if the Discard are Reverse-Loot effects)
next thing is something for AI.
while testing how CounterMoveAi works, i tested Evolve Trigger.
Specially Simic Fluxmage and Cytoplast Root-Kin because of their different MoveCounter Effects.
while testing i noticed that the AI currently has no logic how to stack Abilities that are put on the stack the same time.
in this example the AI should prefer to resolve Evolve Trigger first before PutCounter(All) Effects, so the creature can benefit from that.
another example would that the AI should prefer Draw Effects before Discard Effects (even if the Discard are Reverse-Loot effects)
Re: Hanmac's Refactoring Thread
by Agetian » 22 Dec 2016, 05:21
I agree that since the AI can't really make strategic considerations and operates on a purely tactical (decision-by-decision) basis, then common sense logic should be applied to ordering abilities entering the stack at the same time when possible (Evolve before PutCounter, Draw before Discard, etc.). I think the AI would benefit from that. Even though it won't be universal, at least it'll fit most typical scenarios where the AI would commonly misplay.
- Agetian
- Agetian
- Agetian
- Programmer
- Posts: 3472
- Joined: 14 Mar 2011, 05:58
- Has thanked: 677 times
- Been thanked: 561 times
Re: Hanmac's Refactoring Thread
by Hanmac » 22 Dec 2016, 07:03
Because a simple sort might to complex, I will split the list by APIType first and put them together later. For that I will add a SpellAbilityPredicates helper like the others.
For Draw vs Discard:
If mandatory Discard do it first if hand is empty.
Then do Draw (first the ones without discard subAbility)
Then do Loot (Draw -> Discard)
Then do optional Discard (because it's most likely ReverseLoot)
Then do mandatory Discard if not done before.
For Draw vs Discard:
If mandatory Discard do it first if hand is empty.
Then do Draw (first the ones without discard subAbility)
Then do Loot (Draw -> Discard)
Then do optional Discard (because it's most likely ReverseLoot)
Then do mandatory Discard if not done before.
Re: Hanmac's Refactoring Thread
by Agetian » 22 Dec 2016, 11:21
Sounds reasonable, but maybe we should also think about what to do on a random discard (in that case it's questionable whether it's better to draw first and then random-discard or the other way around; not as straightforward of a choice, anyway; probably in most cases you'd want to random-discard first and then draw something new, hoping that it'll make a difference, but the AI doesn't evaluate its hand vs. a potential possible draw in any way, so this line of thinking does not apply to it. :/ Also, if you have zero cards in hand, then a discard effect should be of higher priority to draw since you won't have to discard a card you've just drawn - this actually applies to both standard discard and random discard).Hanmac wrote:Because a simple sort might to complex, I will split the list by APIType first and put them together later. For that I will add a SpellAbilityPredicates helper like the others.
For Draw vs Discard:
If mandatory Discard do it first if hand is empty.
Then do Draw (first the ones without discard subAbility)
Then do Loot (Draw -> Discard)
Then do optional Discard (because it's most likely ReverseLoot)
Then do mandatory Discard if not done before.
- Agetian
- Agetian
- Programmer
- Posts: 3472
- Joined: 14 Mar 2011, 05:58
- Has thanked: 677 times
- Been thanked: 561 times
Re: Hanmac's Refactoring Thread
by Hanmac » 25 Dec 2016, 14:25
i did commit the first thing with Evolve/PutCounter and Draw/Discard Effects.
for that i added some SpellAbilityPredicates as helper. (that can be useful somewhere too)
there is also a filterList helper function. it does remove the filtered elements from the input list. I use it to have the filtered elements first, and the ones i do not care later.
But i don't know if i should move that to a more common place.
i don't know if there should be more API checks, or more special cases like if the hand is empty or it has DiscardMe cards.
New Project:
Refactoring ReplacementEffects, specially if they are doubling a value like damage or counters or tokens, it might be easier for the Replacement effects to just change the values and have the original function handle the rest.
that could fix the problem with lifelink and (battle)damage manipulation
for that i might add an extra Effect named ReplaceVar.
as Example for Gisela, Blade of Goldnight it would look like that:
what do you guys think about the plan?
it would require some more rewrites because the effects which does call the ReplacementHandler does need to be updated too, to understand what is going on, but i think it would help the code.
for that i added some SpellAbilityPredicates as helper. (that can be useful somewhere too)
there is also a filterList helper function. it does remove the filtered elements from the input list. I use it to have the filtered elements first, and the ones i do not care later.
But i don't know if i should move that to a more common place.
i don't know if there should be more API checks, or more special cases like if the hand is empty or it has DiscardMe cards.
New Project:
Refactoring ReplacementEffects, specially if they are doubling a value like damage or counters or tokens, it might be easier for the Replacement effects to just change the values and have the original function handle the rest.
that could fix the problem with lifelink and (battle)damage manipulation
for that i might add an extra Effect named ReplaceVar.
as Example for Gisela, Blade of Goldnight it would look like that:
- Code: Select all
R:Event$ DamageDone | ActiveZones$ Battlefield | ValidSource$ Card | ValidTarget$ Opponent,Permanent.OppCtrl | ReplaceWith$ DmgTwice | Description$ If a source would deal damage to an opponent or a permanent an opponent controls, that source deals double that damage to that player or permanent instead.
SVar:DmgTwice:DB$ ReplaceEffect | VarName$ DamageAmount | VarValue$ X | References$ X
SVar:X:ReplaceCount$DamageAmount/Twice
what do you guys think about the plan?
it would require some more rewrites because the effects which does call the ReplacementHandler does need to be updated too, to understand what is going on, but i think it would help the code.
Re: Hanmac's Refactoring Thread
by Agetian » 25 Dec 2016, 15:00
Personally I really appreciate the effort here and I think this is a very good idea. It will both streamline the code (and also remove the need to distinguish some special cases in code) and also allow the game to work properly in certain cases where currently the game does not follow the rules and behaves strangely. Best of luck!
- Agetian
- Agetian
- Agetian
- Programmer
- Posts: 3472
- Joined: 14 Mar 2011, 05:58
- Has thanked: 677 times
- Been thanked: 561 times
Re: Hanmac's Refactoring Thread
by Hanmac » 26 Dec 2016, 13:41
i did the ReplaceEffect. tested with addCounter and Hardened Scales
it also uses calculateAmount, for calcing the new value, that might not work yet for other changes. (like damage redirection)
- Code: Select all
R:Event$ AddCounter | ActiveZones$ Battlefield | ValidCard$ Creature.YouCtrl | ValidCounterType$ P1P1 | ReplaceWith$ AddOneMoreCounters | Description$ If one or more +1/+1 counters would be placed on a creature you control, that many plus one +1/+1 counters are placed on it instead.
SVar:AddOneMoreCounters:DB$ ReplaceEffect | VarName$ CounterNum | VarValue$ X | References$ X
SVar:X:ReplaceCount$CounterNum/Plus.1
it also uses calculateAmount, for calcing the new value, that might not work yet for other changes. (like damage redirection)
Re: Hanmac's Refactoring Thread
by Agetian » 26 Dec 2016, 16:29
Ah, this is very nice, Hanmac! Great to hear that this plan is coming to fruition. Is there a possibility to adapt this for use in the lifelink scenario vs. damage replacement?
- Agetian
- Agetian
- Agetian
- Programmer
- Posts: 3472
- Joined: 14 Mar 2011, 05:58
- Has thanked: 677 times
- Been thanked: 561 times
Re: Hanmac's Refactoring Thread
by Hanmac » 26 Dec 2016, 17:08
i updated TokenDoubler now as keyword using Replacement Effect.
with that i can finally get Varchild's War-Riders as real Cumulative upkeep working.
for that i planed a cost like that:
for the lifelink scenario i will look, there are many more cards than just with the Token Doublers
with that i can finally get Varchild's War-Riders as real Cumulative upkeep working.
for that i planed a cost like that:
- Code: Select all
CreateToken<1/Player.Opponent/1/1/Red/Creature,Survior>
for the lifelink scenario i will look, there are many more cards than just with the Token Doublers
- Agetian
- Programmer
- Posts: 3472
- Joined: 14 Mar 2011, 05:58
- Has thanked: 677 times
- Been thanked: 561 times
Re: Hanmac's Refactoring Thread
by Agetian » 27 Dec 2016, 18:30
Hmm, for some reason Android compilation was broken and it's complaining about a method in the newly introduced ReplaceEffect
(I may be wrong but I think the Android SDK we're targeting is using Java 7 era libraries).
Would this, for example, suffice as an alternative?
- Code: Select all
[INFO] Warning: forge.game.ability.effects.ReplaceEffect: can't find referenced method 'java.lang.Object replace(java.lang.Object,java.lang.Object)' in class java.util.Map
(I may be wrong but I think the Android SDK we're targeting is using Java 7 era libraries).
Would this, for example, suffice as an alternative?
- Code: Select all
if (originalParams.containsKey(e.getKey())) {
originalParams.put(e.getKey(), e.getValue());
}
- Agetian
- Programmer
- Posts: 3472
- Joined: 14 Mar 2011, 05:58
- Has thanked: 677 times
- Been thanked: 561 times
Re: Hanmac's Refactoring Thread
by Hanmac » 27 Dec 2016, 19:05
Hm normally a simple put should be enough because the map should not have parameters with where not there before.
(You can replace "replace" with just "put")
I wanted to do a map.replace (newMap) but there is no nice function for that.
(You can replace "replace" with just "put")
I wanted to do a map.replace (newMap) but there is no nice function for that.
54 posts
• Page 1 of 4 • 1, 2, 3, 4
Who is online
Users browsing this forum: No registered users and 104 guests