[fixed] Bazaar of Baghdad
Moderators: BAgate, drool66, Aswan jaguar, gmzombie, stassy, CCGHQ Admins
[fixed] Bazaar of Baghdad
by joseluiselblanco » 10 Nov 2021, 15:07
Describe the Bug:
Not sure it's a bug but when I double click to automatically pay the cost of a spell, if I have a Baazar of Baghdad out, it taps the Baazar of Baghdad even though it doesn't produce any mana.
Which card did behave improperly?
Bazaar of Baghdad
Which update are you using? (date, name)Which type? (duel, gauntlet, sealed deck)
Latest version, duel
Not sure it's a bug but when I double click to automatically pay the cost of a spell, if I have a Baazar of Baghdad out, it taps the Baazar of Baghdad even though it doesn't produce any mana.
Which card did behave improperly?
Bazaar of Baghdad
Which update are you using? (date, name)Which type? (duel, gauntlet, sealed deck)
Latest version, duel
Last edited by drool66 on 30 Nov 2021, 02:48, edited 2 times in total.
Reason: fixed, I think
Reason: fixed, I think
- joseluiselblanco
- Posts: 47
- Joined: 15 Apr 2020, 12:39
- Has thanked: 11 times
- Been thanked: 0 time
Re: Bazaar of Baghdad
by Korath » 10 Nov 2021, 16:19
Yes, this is a bug.
You can't start flagging non-mana-sources as EA_MANA_SOURCE without A) telling the mana payment AI that they aren't really, and B) making their non-mana activated abilities unactivateable during mana payment. Otherwise - aside from the irritating autotapping behavior reported here - you can do illegal things like using Bazaar of Baghdad to discard a card you're in the middle of casting (and fail, but not get the other mana you spent on it back) or Diamond Valley to sacrifice a creature like Centaur's Herald while activating its ability with a sacrifice-self cost (and succeed), and you'll make the AI not consider casting spells or activating abilities because it doesn't like that it has to activate those non-mana sources too as a side effect.
The proper place for A is in can_autotap() at 0x42fda0, which you'll have to move out of magic.exe - a good idea anyway, since it still doesn't consider Wastes or snow-covered lands basic. Let me know if you need help; but don't try using the Shandalar version, which has different semantics.
B has to be done in every misflagged card's function, or, much better, something they all call.
You can't start flagging non-mana-sources as EA_MANA_SOURCE without A) telling the mana payment AI that they aren't really, and B) making their non-mana activated abilities unactivateable during mana payment. Otherwise - aside from the irritating autotapping behavior reported here - you can do illegal things like using Bazaar of Baghdad to discard a card you're in the middle of casting (and fail, but not get the other mana you spent on it back) or Diamond Valley to sacrifice a creature like Centaur's Herald while activating its ability with a sacrifice-self cost (and succeed), and you'll make the AI not consider casting spells or activating abilities because it doesn't like that it has to activate those non-mana sources too as a side effect.
The proper place for A is in can_autotap() at 0x42fda0, which you'll have to move out of magic.exe - a good idea anyway, since it still doesn't consider Wastes or snow-covered lands basic. Let me know if you need help; but don't try using the Shandalar version, which has different semantics.
B has to be done in every misflagged card's function, or, much better, something they all call.
-
Korath - DEVELOPER
- Posts: 3707
- Joined: 02 Jun 2013, 05:57
- Has thanked: 496 times
- Been thanked: 1106 times
Re: Bazaar of Baghdad
by drool66 » 11 Nov 2021, 19:48
Yep, I did this back before I knew the consequences of anything.
B is no trouble.
Does this look right?
B is no trouble.
Does this look right?
- Code: Select all
int can_autotap(int player, int card, int autotap_flags)
{
// 0x42fda0
card_instance_t* inst = in play(player, card);
if( !inst )
return 0;
iid_t iid = inst->internal_card_id;
color_test_t colors = (inst->card_color & (COLOR_TEST_ANY | COLOR_TEST_ARTIFACT));
if( is_what(player, card, TYPE_LAND) )
colors |= mana_colors_added[player];
if ( colors <= COLOR_TEST_0
|| inst->state & STATE_TAPPED
|| !(cards_data[iid].extra_ability & EA_MANA_SOURCE || (is_what(player, card, TYPE_LAND) && mana_colors_added[player] > COLOR_TEST_0))
|| cards_data[iid].extra_ability & EA_PAID_MANA_SOURCE ) //huh, really?
return 0;
if ( (autotap_flags & AUTOTAP_NO_BASIC_LANDS) && is_basic_land(player, card) )
return 0;
if ( (autotap_flags & AUTOTAP_NO_NONBASIC_LANDS) && is_what(player, card, TYPE_LAND) && !is_basic_land(player, card) )
return 0;
if ( (autotap_flags & AUTOTAP_NO_DONT_AUTO_TAP) && (inst->state & STATE_NO_AUTO_TAPPING) )
return 0;
if ( (autotap_flags & AUTOTAP_NO_ARTIFACTS) && is_what(player, card, TYPE_ARTIFACT) )
return 0;
if ( (autotap_flags & AUTOTAP_NO_CREATURES) && is_what(player, card, TYPE_CREATURE) )
return 0;
return 1;
}
The latest images for Manalink will be here.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
-
drool66 - Programmer
- Posts: 1163
- Joined: 25 Nov 2010, 22:38
- Has thanked: 186 times
- Been thanked: 267 times
Re: Bazaar of Baghdad
by Korath » 11 Nov 2021, 20:48
- Code: Select all
if ( colors <= COLOR_TEST_0
- Code: Select all
|| inst->state & STATE_TAPPED
I very vaguely recall this being problematic at one point. We may well have patched it out before we started keeping track of the changes we made directly to magic.exe. There's no such check in the current version nor has been for years, just in case the decompilation you're reading has it.
- Code: Select all
|| !(cards_data[iid].extra_ability & EA_MANA_SOURCE || (is_what(player, card, TYPE_LAND) && mana_colors_added[player] > COLOR_TEST_0))
- Code: Select all
|| cards_data[iid].extra_ability & EA_PAID_MANA_SOURCE ) //huh, really?
- Code: Select all
if ( (autotap_flags & AUTOTAP_NO_NONBASIC_LANDS) && is_what(player, card, TYPE_LAND) && !is_basic_land(player, card) )
-
Korath - DEVELOPER
- Posts: 3707
- Joined: 02 Jun 2013, 05:57
- Has thanked: 496 times
- Been thanked: 1106 times
Re: [confirmed] Bazaar of Baghdad
by drool66 » 11 Nov 2021, 23:12
Yes - the idea is to remove EA_MANA_SOURCE from lands with no mana abils that I added in 50611a9eb ("Lands with added mana production can be tapped directly, +Riftstone Portal", 2020-08-24), and also add this same kind of check in count_mana(). The point is to allow them to be autotapped and declare their mana available with an Urborg, Tomb of Yawgmoth et al. in play. I'm not sure if that will gum up anything else though.It's hard to imagine a card that would pass the right half of the logical-or without passing the left, unless you plan on combining this with data changes.
- Code: Select all
|| !(cards_data[iid].extra_ability & EA_MANA_SOURCE || (is_what(player, card, TYPE_LAND) && mana_colors_added[player] > COLOR_TEST_0))
So more like the exe does?This prevents Gem Bazaar and Mishra's Workshop from being preferentially activated, compared to the exe version. (Which is ok if intentional.)
- Code: Select all
if ( (autotap_flags & AUTOTAP_NO_NONBASIC_LANDS) && is_what(player, card, TYPE_LAND) && !is_basic_land(player, card) )
- Code: Select all
if ( (autotap_flags & AUTOTAP_NO_BASIC_LANDS) && (is_basic_land(player, card) || id == CARD_ID_GEM_BAZAAR || inst->mana_color == COLOR_TEST_ARTIFACT) )
return 0;
etc...
The latest images for Manalink will be here.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
-
drool66 - Programmer
- Posts: 1163
- Joined: 25 Nov 2010, 22:38
- Has thanked: 186 times
- Been thanked: 267 times
Re: [confirmed] Bazaar of Baghdad
by Korath » 12 Nov 2021, 12:20
I'd be very wary. Very many things check for EA_MANA_SOURCE for very many reasons. Manastripe display. Drain Power, and Power Sink, and lots of cards like Crackdown Construct that misuse it as a proxy for whether the particular ability was a mana ability. Whether they can be activated while mana is being charged, and whether activation can be responded to. AI valuation. They're all a problem going in the other direction, too, but generally a more mitigable one - Strip Mine's always been there, for example, so there's almost always an out after the EA_MANA_SOURCE check to exclude a card.drool66 wrote:Yes - the idea is to remove EA_MANA_SOURCE from lands with no mana abils that I added in 50611a9eb ("Lands with added mana production can be tapped directly, +Riftstone Portal", 2020-08-24), and also add this same kind of check in count_mana(). The point is to allow them to be autotapped and declare their mana available with an Urborg, Tomb of Yawgmoth et al. in play. I'm not sure if that will gum up anything else though.
Well, you've already seen what trying to special-case everything does to mana cost modification.Any better way to do that other than those two highly specific checks, maybe like "!(cards_data[iid].extra_ability & EA_ACTIVATE)" or something?
EA_ACT_ABILITY won't work either. Consider Lake of the Dead and Ruins of Trokair: all abilities are mana abilities, so they should be EA_MANA_SOURCE and not EA_ACT_ABILITY; but you want to delay activating them if you can, so you can see whether you need to use their more expensive ability. Ghost Town is an example in the other direction - it does have an activated, non-mana ability, but tapping it for mana doesn't make any difference in whether you can bounce it. And cards like Exotic Orchard or Meteor Crater are reasonably likely to have more options the longer you put off activating them.
The usual solution to this kind of problem is to ask the object what to do, and if you look at Shandalar's version of can_autotap(), you'll see that's what I settled on. It's ok that there are surely mana sources that don't respond to QUERY_IS_PLAIN_MANA_PRODUCER; it just means the AI doesn't perform as well as it could, and as long as the five original basics and Mishra's Workshop (and Gem Bazaar, I suppose, if you're completist enough) do respond, you're not any worse off than you were before.
-
Korath - DEVELOPER
- Posts: 3707
- Joined: 02 Jun 2013, 05:57
- Has thanked: 496 times
- Been thanked: 1106 times
Re: [confirmed] Bazaar of Baghdad
by drool66 » 30 Nov 2021, 02:48
I believe this is fixed in adf2081 & 286dc33
The latest images for Manalink will be here.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
The latest Manalink installation directory will be here. Well, not quite, anymore. Check the latest patches.
-
drool66 - Programmer
- Posts: 1163
- Joined: 25 Nov 2010, 22:38
- Has thanked: 186 times
- Been thanked: 267 times
7 posts
• Page 1 of 1
Who is online
Users browsing this forum: No registered users and 28 guests