Page 1 of 1

Assorted bugs, updated any time I generate new info

PostPosted: 28 Mar 2013, 15:28
by Dragoonix
ABILITIES
Madness - This ability doesn't seem to be able to trigger off a standard discard phase discard

Buyback - I read that this periodically doesn't return the spell when paid, I may have witnessed this once, tho I wasn't paying as much attention as I should have been

CARD SPECIFIC
Brighthearth Banneret - the reinforce ability on this card seems to be absent

Desecration Demon - Seems to be coded so that he is tapped and a counter added at the start of every combat whether something is sacrificed or not, but only if he already has a +1/1 counter

Animate Dead - The AI managed to cast this on one of my creatures currently in play resulting in full effects as though it had been in the grave. Not sure if it's possible by a human player

Elven Riders - These presumably need to be recoded to accept creatures with 'defender' as walls, not a true error, but would be much nicer this way

Weatherseed Totem - Converting this to a creature with its built in ability has a 100% crash rate so far

Blistercoil Weird - tooltip misreads this card as getting +2/2 on spell cast, should be and IS coded as +1/1

Experiment One - strange things happen with his toughness when he uses his ability to regenerate

Fertile Ground - not sure if this can be fixed, for purposes of 'auto' fertile ground isn't recognized as a source of mana, or at least a source of any color mana, not sure which (it can assume you have no valid options and pass when in fact you could cast w/ the mana provided by fertile ground)

Dawn's Reflection - this card seems to do nothing when it resolves you get a line in the log about it resolving when you tap the land, but nothing actually happens(3G enchant land, when enchanted land is tapped for mana it's controller adds to mana of any combination of colors to his mana pool)

Orzhov Charm - The 'return target creature you control and all auras you control on it to owners hand' doesn't return auras

Re: Assorted bugs, updated any time I generate new info

PostPosted: 08 Apr 2013, 06:12
by Marek14
Dragoonix wrote:ABILITIES
Madness - This ability doesn't seem to be able to trigger off a standard discard phase discard

Buyback - I read that this periodically doesn't return the spell when paid, I may have witnessed this once, tho I wasn't paying as much attention as I should have been

CARD SPECIFIC
Brighthearth Banneret - the reinforce ability on this card seems to be absent

Desecration Demon - Seems to be coded so that he is tapped and a counter added at the start of every combat whether something is sacrificed or not, but only if he already has a +1/1 counter

Animate Dead - The AI managed to cast this on one of my creatures currently in play resulting in full effects as though it had been in the grave. Not sure if it's possible by a human player

Elven Riders - These presumably need to be recoded to accept creatures with 'defender' as walls, not a true error, but would be much nicer this way

Weatherseed Totem - Converting this to a creature with its built in ability has a 100% crash rate so far

Blistercoil Weird - tooltip misreads this card as getting +2/2 on spell cast, should be and IS coded as +1/1

Experiment One - strange things happen with his toughness when he uses his ability to regenerate

Fertile Ground - not sure if this can be fixed, for purposes of 'auto' fertile ground isn't recognized as a source of mana, or at least a source of any color mana, not sure which (it can assume you have no valid options and pass when in fact you could cast w/ the mana provided by fertile ground)

Dawn's Reflection - this card seems to do nothing when it resolves you get a line in the log about it resolving when you tap the land, but nothing actually happens(3G enchant land, when enchanted land is tapped for mana it's controller adds to mana of any combination of colors to his mana pool)

Orzhov Charm - The 'return target creature you control and all auras you control on it to owners hand' doesn't return auras
OK, let me see. Blistercoil Weird -- just a simple typo in the text file. Fixed.
Brighthearth Banneret -- also simple, the cost for reinforce was miscoded as {1W} instead of {1R}.
Elven Riders -- Wizards policy is no power-level errata -- sorry!
Fertile Ground -- you generally need to tap your mana before casting spell in BotArena.
Experiment One -- this one might be a result of the way temporary and permanent changes to P/T are displayed. Is there a bug in how it WORKS, or does it just LOOK weird?

Desecration Demon -- this card haunts me (ironically more than true cards with haunt). Here, I'm going to post the code with comments -- it's also a chance to show you how BotArena works.
BTW, I managed to find the bug during commenting :)

Code: Select all
CDesecrationDemonCard::CDesecrationDemonCard(CGame* pGame, UINT nID)
   : CFlyingCreatureCard(pGame, _T("Desecration Demon"), CardType::Creature, CREATURE_TYPE(Demon), nID,
      _T("2") BLACK_MANA_TEXT BLACK_MANA_TEXT, Power(6), Life(6))
// This is standard header. It defines name, card type and mana cost, and for creatures it also sets their type and printed P/T. CFlyingCreatureCard is one of several classes for commonly-used creatures so flying doesn't need to be defined separately.
   , m_PunisherSelection(pGame, CSelectionSupport::SelectionCallback(this, &CDesecrationDemonCard::OnPunisherSelected))
// Desecration Demon contains a selection code (opponent should choose whether to sacrifice creature or not). This line defines which function is to be used to resolve the selection.
   , bSomeonePaid(0)
// This is a variable stored on the card that says whether someone paid the cost or not. Even though BotArena currently only supports two players, I tried to make cards multiplayer-friendly wherever possible.

{
   typedef
      TTriggeredAbility< CTriggeredAbility<>, CWhenNodeChanged > TriggeredAbility;
// Header of the triggered ability. This is a CWhenNodeChanged triggered ability, meaning it triggers when a specific phase or step starts.

   counted_ptr<TriggeredAbility> cpAbility(
      ::CreateObject<TriggeredAbility>(this, NodeId::BeginningOfCombatStep));
// Further clarification of the trigger: it says that it triggers at the beginning of combat.

   ATLASSERT(cpAbility);

   cpAbility->SetOptionalType(TriggeredAbility::OptionalType::Required);
// Important consideration. Default in BotArena is to make triggered abilities optional. Whenever they are required and can't be declined, this line is a must.
   
   cpAbility->SetResolutionStartedCallback(CAbility::ResolutionStartedCallback(this, &CDesecrationDemonCard::BeforeResolution));
// Since Desecration Demon's ability is too complex to fit into any of the predefined boxes, I had to write my own code for it. Standardly, you get an option to call two functions for resolution of a spell or ability, one before the standard code of the ability is executed, and one after. This ability is a "dummy" one of class CTriggeredAbility<>, so it has no standard code to execute, making these two approaches basically equivalent.
   AddAbility(cpAbility.GetPointer());
}

bool CDesecrationDemonCard::BeforeResolution(CAbilityAction* pAction)
//This is the meat of the ability.
{
   CPlayer* pActivePlayer = GetGame()->GetActivePlayer();
   int pActivePlayerID;
   for (int ip = 0; ip < GetGame()->GetPlayerCount(); ++ip)
      if (pActivePlayer == GetGame()->GetPlayer(ip))
      {
         pActivePlayerID = ip;
         break;
      }
// Here, I read the active player, then iterate through players until I find his ID number. I never found a way to get player's ID directly.

   PunisherFunction(pActivePlayerID);
// And here, I call the main function of this resolution...

   return true;
}

void CDesecrationDemonCard::PunisherFunction(int PlayerID)
// ...which is defined here. Now, this is complicated.
{
   CPlayer* pPlayer = GetGame()->GetPlayer(PlayerID);
   CZone* pBattlefield = pPlayer->GetZoneById(ZoneId::Battlefield);
   CCardFilter m_CardFilter;
   m_CardFilter.SetComparer(new AnyCreatureComparer);
// This sets basic information. There is a player who is about to sacrifice a creature (in the beginning, this is the active player to comply with the standard order in Magic). I also define that player's battlefield zone and a filter that controls what he can sacrifice: a creature.

   if (pPlayer != GetController() && m_CardFilter.CountIncluded(pBattlefield->GetCardContainer()) > 0)
// The condition stops the sacrifice if a) the player is controller of Desecration Demon (should probably change that to controller of the ability, since this part could cause weirdness if control changes between announcement and resolution of the ability), or b) the player doesn't control any creatures (and so can't sacrifice).
   {
      std::vector<SelectionEntry> entries;
// Here we start to define the selections
      {
         SelectionEntry selectionEntry;

         selectionEntry.dwContext = 0;
         selectionEntry.strText.Format(_T("Don't sacrifice anything"));

         entries.push_back(selectionEntry);
      }
// First option, of course, is to not sacrifice anything. Since the sacrifice is optional, this option must be always present.
      for (int i = 0; i < pBattlefield->GetSize(); ++i)
      {
         CCard* pCard = pBattlefield->GetAt(i);
         if (m_CardFilter.IsCardIncluded(pCard))
         {
            SelectionEntry entry;
               entry.dwContext = (DWORD)pCard;
            entry.cpAssociatedCard = pCard;
                        
            entry.strText.Format(_T("Sacrifice %s"),
               pCard->GetCardName(TRUE));
               entries.push_back(entry);
         }
               
      }
// This part iterates through the player's permanents. Any creature that is found results in adding a new option to the selection.
      m_PunisherSelection.AddSelectionRequest(entries, 1, 1, NULL, pPlayer, PlayerID);
// And finally, this line will call resolution function for the selection, which is defined way back in the header.
   }
   else
      Advance(PlayerID);
// On the other hand, if the selection doesn't happen (for either of the two reasons mentioned), the PlayerID is advanced, which is handled by the next function.
}

void CDesecrationDemonCard::Advance(int PlayerID)
{
   int NextPlayer = PlayerID + 1;
   int PlayerCount = GetGame()->GetPlayerCount();
   CPlayer* pActivePlayer = GetGame()->GetActivePlayer();

   if (NextPlayer >= PlayerCount)
      NextPlayer -= PlayerCount;
   if (GetGame()->GetPlayer(NextPlayer) != pActivePlayer)
      PunisherFunction(NextPlayer);
// This increases PlayerID value from parameter by 1, and then reduces it by number of players if it's greater or equal to keep it in (0..number - 1) range. If the new player is the active player, we have looped to the beginning and next part is executed. If not, we just got another player who could sacrifice a creature. Note that just because one opponent sacrifices a creature doesn't preclude another one from doing so.

   else if ((bSomeonePaid > 0) && IsInplay())
// The following code should be only executed if a) someone sacrificed a creature and b) Desecration Demon is still on the battlefield. Aaand it's right about here where I noticed the bug in my code: it's not enough to initialize bSomeonePaid to 0 -- it must be also initialized during the resolution function, otherwise the value will carry between turns: this is what caused the auto-tap when there was already a counter.
   {
      CCardOrientationModifier pModifier1 = CCardOrientationModifier(TRUE);
      CCardCounterModifier pModifier2 = CCardCounterModifier(_T("+1/+1"), 1);
// When BotArena actually does something, it's usually in form of modifiers. The first modifier defines tapping a card, while the second defines adding a +1/+1 counter.

      pModifier1.ApplyTo(this);
      pModifier2.ApplyTo(this);
// And these lines instruct BotArena to apply both modifiers to the source card of the ability.
   }
}

void CDesecrationDemonCard::OnPunisherSelected(const std::vector<SelectionEntry>& selection, int nSelectedCount, CPlayer* pSelectionPlayer, DWORD dwContext1, DWORD dwContext2, DWORD dwContext3, DWORD dwContext4, DWORD dwContext5)
// This is a standard header of selection resolution.
{
   ATLASSERT(nSelectedCount == 1);

   for (std::vector<SelectionEntry>::const_iterator it = selection.begin(); it != selection.end(); ++it)
      if (it->bSelected)
      {
// Basically, you iterate through possible values of the selection until you find the one that is actually selected. You could, theoretically, select multiple values and have all their results happen. This doesn't actually happen in BotArena, AFAIK.
         if ((int)it->dwContext == 0)
         {
            if (!m_pGame->IsThinking())
            {
               CString strMessage;
               strMessage.Format(_T("%s sacrifices nothing"), pSelectionPlayer->GetPlayerName());
               m_pGame->Message(
                  strMessage,
                  pSelectionPlayer->IsComputer() ? m_pGame->GetComputerImage() : m_pGame->GetHumanImage(),
                  MessageImportance::High
                  );
            }
            Advance(dwContext1);

// This is for case when a player sacrifices nothing. This just makes the player ID advance.
            
            return;
         }
         else
         {
            CCard* pCard = (CCard*)it->dwContext;
// This, on the other hand, is more interesting. First of all, we define a card. The value of the option that was selected is actually the pointer to the card, this line restores the card from it.

            if (!m_pGame->IsThinking())
            {
               CString strMessage;
               strMessage.Format(_T("%s sacrifices %s"), pSelectionPlayer->GetPlayerName(), pCard->GetCardName(TRUE));
               m_pGame->Message(
                  strMessage,
                  pSelectionPlayer->IsComputer() ? m_pGame->GetComputerImage() : m_pGame->GetHumanImage(),
                  MessageImportance::High
                  );
            }
            CMoveCardModifier pModifier1 = CMoveCardModifier(ZoneId::Battlefield, ZoneId::Graveyard, TRUE, MoveType::Sacrifice, pSelectionPlayer);
            pModifier1.ApplyTo(pCard);
// Another use of modifiers: this is a move modifier that moves a card. There is also a lower-level way to do this (method Move), but I only use that if the move is particularly strange -- like moving to the bottom of library or moving from opponent's graveyard to your battlefield. The Move Modifier has several important parameters: first two define zones the card is moving from and to, and as an error-catching mechanism, the move will fail if the card is not actually in the "from" zone. The TRUE refers to question whether both zones are owned by the same player. In my quest for multiplayer-friendly code, I don't use FALSE here -- instead, I generally use the Move method that allows more precise specification of the zones. MoveType says what kind of move this is. There are several different kinds. Destroy, DestroyWithoutRegeneration, Discard, etc., mostly useful for triggered abilities that care about specific kind of moving (and for regeneration/totem armor/indescructibility that can stop one kind of battlefield/graveyard move, but not others). In this case, it's sacrifice. BTW, I'm actually considering adding Devour as a specific type of moving, since that seems the only way to implpement Kresh the Bloodbraided Avatar (a Vanguard card). But since the move types cannot be combined, this would mean I'd have to edit all cards that currently trigger on sacrificing to also trigger on devouring.
// The last parameter says who performs the move. In this case, it's the player who selected the creature to sacrifice.
// A fun fact -- up until recently, BotArena didn't have the rule "you can only sacrifice things you control" implemented. I added that -- that was the reason why sacrifice was broken. Other coders didn't use the last parameter the same way as me, leading to conflicts. The rule wasn't actually that necessary up until recently, but I found that its absence was causing weird problems in corner cases. For example, if you activate Deathknell Kami and then donate it to opponent, it shouldn't die at end of turn: the instructions say you should sacrifice it, but since you don't control it, you can't. Under old system, it was sacrificed nonetheless -- that's why I added the hard rule that says moving fails if MoveType is Sacrifice and moving player is anyone but the current controller.

            bSomeonePaid = 1;
// Here, the flag is set. Someone sacrificed a creture, therefore Desecration Demon will be tapped. Eventually.
            Advance(dwContext1);
// And like in the first case, we advance the PlayerID.

            return;
         }
      }
}

// And we're done! I hope this was interesting.

//____________________________________________________________________________
//
So, I attach the corrected dlls which should fix Blistercoil Weird, Desecration Demon and Brighthearth Banneret.

Re: Assorted bugs, updated any time I generate new info

PostPosted: 08 Apr 2013, 06:13
by Marek14
Um, just backup your old files before using these, though. Theres a chance that they won't be compatible with current released version of MagicCore.dll and will crash your game, and if that happens, we'll have to release all dlls at once. Just a note.

I'm off to code some Dragon's Maze :)

Re: Assorted bugs, updated any time I generate new info

PostPosted: 11 Apr 2013, 21:52
by Dragoonix
Quickly want to thank you for the work, all seems in order no issues with compatibility.

Have to admit, I've seen some walls of text, but having a chunk of code pasted in there probably takes the cake :D while my coding skills are super rusty -10 at best, I am at heart a logic buff. I saw like 2 lines and determined exactly what was happening, the 'cost paid' wasn't ever being reset so every time it checked to see if the cost was paid in the future, it said, yes, yes it has been paid.

On a side note, one of the biggest issues I have with the program is multi targeting things can be a major pain to find the right option, but then you cast something like rewind and it just lets you conveniently select 1 by 1 until you are done. I suspect this has something to do with untapping lands being a special case? (I believe it actually resolves each untap as you select it whereas in reality they would all untap at once)

Champion x - I used this ability to champion the creature made as a result of activating a keyrune. When the champion creature left play, the keyrune was not returned, this is a mistake as far as i can read the rules. For complete disclosure sake (Changeling berserker champions rakdos keyrune, berserker is later sacrificed to a ragamuffyn)

Regeneration // Experiment One - Turns out this may be a more general regeneration issue. Exact example I can give atm, ++2/2rrr sporeback troll blocked by 8/8, regenerates to have 4(2) toughness. Come to think this may be an issue with +1/1 counters and regeneration, haven't seen issues regenerating either mortivor or korlash, heir to black blade. At best guess, it's reading the creatures toughness (including all counters on it) then 'resetting' the toughness to that, then slapping the counters on again.

CARDS

Chandra Naalar - her 2nd ability -x's basic targeting is your own creatures, you have to use all options to target enemy creatures, not a true bug I suppose, but not the way you would have intended I'm sure.

Re: Assorted bugs, updated any time I generate new info

PostPosted: 12 Apr 2013, 05:24
by Marek14
If you select things on resolution, it's possible to select them one by one. I know how to write Rewind so the cards would be untapped all at once (or, at least, so they would LOOK like that), I'd just need to define a container, add all selected cards to it, then apply untap modifier on them all.

For true targets, this is harder. The main problem is that BotArena doesn't split the announcement of spell or ability into steps, so everything has to be specified at once. Some cards are impractical because of this.

Champion bug is, of course, caused by the animation mechanics we use. Since animated forms are pseudocards in their own rights with a whole of special rules to weed out bugs (you move them together with normal cards, you make them share counters, etc.) this particular problem isn't addressed at this time.

As for Chandra, I'm adding a line "cpAbility->GetTargeting()->SetDefaultCharacteristic(Characteristic::Negative);" to its code to affect situations where you remove at least one counter for its second ability, this makes it preferentially target opponent's stuff.

Re: Assorted bugs, updated any time I generate new info

PostPosted: 13 Apr 2013, 17:39
by Dragoonix
'some cards are impractical' I love it lol. You'll find however that I am a stubborn old goat and will route through 9000 options to use the card I have selected, I'll simply pass the time grumbling at having to find the correct option. In more 'meta' style magic games the board rarely gets cluttered afaik so less of an issue there. ATM I'm playing exclusively out of decks I personally own which are a mish mash of old and new and nothing with a passion for fancy spells that no one in their right mind would ever consider 'complete' it's just how I roll I guess.

REGENERATION
As for regeneration mishaps, I think I can confirm at this point that it is regeneration with +1/1 counters that is causing the issue. Their toughness is reset to their 'current max' then any +1/1 counters add on on top of that. [a 2/2 with 3 counters would regen to 4/7(4)] some added math in the regeneration section should be able to fix it, just run a subtraction of x toughness where x is equal to the number of +'s on the creature. I'll leave it to the professionals to make sure this isn't accidentally killing off the creature (sporeback troll is a good card to test on as he is exclusively made up of +1/1's and has built in regeneration) Really an extremely minor issue overall as it requires both regeneration and + counters to be present on top of being able to manipulate the creature as a result of their toughness after they are regenerated. Just thought I'd keep the info flowing.

Cheers, and Thanks, as always.

Doubt it would come up, but should the logic behind how to code something, rather then the actual coding itself, ever cause you trouble and you want an extra view point, feel free to shoot me an email.

PS if there is anything you think i could generate to help you fix the crashes resulting from 'weatherseed totem' please do let me know, it's a card my treefolk deck is quite sad to lose as it's a corner stone of them actually getting off their butts and hurting something as opposed to just building a forest.

Re: Assorted bugs, updated any time I generate new info

PostPosted: 14 Apr 2013, 07:14
by Marek14
I tried Weatherseed Totem out. Fortunately, it turned out to be a simple bug. I forgot to update the code when I was reworking token system so it tried to create an animation token that no longer exists. I updated the code with correct token data. The result is attached.

Re: Assorted bugs, updated any time I generate new info

PostPosted: 15 Apr 2013, 14:26
by Dragoonix
Marek14 wrote:I tried Weatherseed Totem out. Fortunately, it turned out to be a simple bug. I forgot to update the code when I was reworking token system so it tried to create an animation token that no longer exists. I updated the code with correct token data. The result is attached.
Greatness at the speed of awesomeness, many thanks my good man.

What with you fixing everything at the drop of a hat it seems a shame that I'm running out of ideas. I'm not sure how much ability you have to mod the core of the game/AI, but something that comes to mind is it would be nice if the computer either by default or by option, played with the 'auto' function running so that he doesn't spend time thinking when he literally has no viable options.

I find that he plays quite quickly until about mid game, then he seems to decide that he needs to be in deep thought about every nook and cranny, only to run 1/2 of his creatures into an easy clean block on the players end.

Re: Assorted bugs, updated any time I generate new info

PostPosted: 22 Apr 2013, 05:33
by Dragoonix
Just a quick one.

Footbottom feast - you don't get any control over the order when you move them to top of your library. just need to splice in a section where you can reorder x top cards of your library between the phase where it tops them and you draw a card, where x is the number of cards moved.

phew, ok yea quickly confirmed you should get to re-order them.

Re: Assorted bugs, updated any time I generate new info

PostPosted: 27 Apr 2013, 01:19
by Dragoonix
Unleash ability - This seems to trigger on its own some times, the computer even had it happen to him, the log shows him selecting not to add a counter, but there he is, with a counter on him.

Rushing-tide zubera - just had this blocked by an elven riders (3/3) w/o any additional dmg it resulted in triggering the 3 card draw on zubera.

Izzet Keyrune - The creature doesn't trigger the 'may' condition on its ability.

Vampire Nocturnus - Not quite sure how to classify this one, its not so much specifically with him as with when cards with triggers check those triggers. I top decked a black spell with a liliana vess but his ability never rechecked the card to grant his bonus. Similar issues with keldon warlord style creatures and animated artifacts. Not sure on how to blanket fix, seems like each instance needs to be more carefully coded for similar triggers. I'm not sure if you could code something that says 'x ability adjusts y' and have all triggers dependent on y recheck on any such y adjustments. Things like animated artifacts are odd, because they cannot be counted as a creature coming into play, but they do count t-wards number of creatures in play.

Also, this could use a thread move over to card bugs I guess since that's what it has mostly become.