[fixed]Treva, the Renewer 's ability doesn't trigger
Moderators: BAgate, drool66, Aswan jaguar, gmzombie, stassy, CCGHQ Admins
[fixed]Treva, the Renewer 's ability doesn't trigger
by Aswan jaguar » 02 Dec 2017, 11:04
Describe the Bug:
Treva, the Renewer 's ability doesn't trigger whenever it deals combat damage to a player.
Which card did behave improperly?
Treva, the Renewer
Which update are you using? (date, name)Which type? (duel, gauntlet, sealed deck)
Manalink dev 778ccb5 version - duel
What exactly should be the correct behavior/interaction?
Are any other cards possibly affected by this bug?
Similarly bugged to Darigaaz, the Igniter here:
viewtopic.php?f=86&t=15397#p220860
EDIT: The bug affects all invasion set dragons.
Treva, the Renewer 's ability doesn't trigger whenever it deals combat damage to a player.
Which card did behave improperly?
Treva, the Renewer
Which update are you using? (date, name)Which type? (duel, gauntlet, sealed deck)
Manalink dev 778ccb5 version - duel
What exactly should be the correct behavior/interaction?
Are any other cards possibly affected by this bug?
Similarly bugged to Darigaaz, the Igniter here:
viewtopic.php?f=86&t=15397#p220860
EDIT: The bug affects all invasion set dragons.
- Attachments
-
- treva trigger;.rar
- (2.54 KiB) Downloaded 261 times
Last edited by Aswan jaguar on 13 Aug 2020, 17:50, edited 3 times in total.
Reason: fixed
Reason: fixed
---
Trying to squash some bugs and playtesting.
Trying to squash some bugs and playtesting.
-
Aswan jaguar - Super Tester Elite
- Posts: 8078
- Joined: 13 May 2010, 12:17
- Has thanked: 730 times
- Been thanked: 458 times
Re: Treva, the Renewer 's ability doesn't trigger
by gnomefry » 02 Mar 2020, 22:38
Confirming that Treva's combat ability still doesn't trigger.
Re: [comfirmed]Treva, the Renewer 's ability doesn't trigger
by FastEddie » 13 Aug 2020, 15:24
This is an interesting one and I hope that someone can enlighten me why things are the way they are. A quick fix might be feasible but I don't want to break something.
The issue is within the invasion_dragon function which is called by all legendary invasion dragons:
The issue is within the invasion_dragon function which is called by all legendary invasion dragons:
- Code: Select all
static void invasion_dragon(int player, int card, event_t event, int cless, int black, int blue, int green, int red, int white){
if (damage_dealt_by_me(player, card, event, DDBM_MUST_DAMAGE_PLAYER | DDBM_TRACE_DAMAGED_PLAYERS | DDBM_MUST_BE_COMBAT_DAMAGE))
{
card_instance_t* instance = get_card_instance(player, card);
int times_damaged[2] = { BYTE0(instance->targets[1].player), BYTE1(instance->targets[1].player) };
instance->targets[1].player = 0;
APNAP(p,{
if( times_damaged[p] ){
char buffer[100];
if( p == player ){
scnprintf(buffer, 100, "Activate %s (you were damaged)", cards_ptr[get_id(player, card)]->name);
}
else{
scnprintf(buffer, 100, "Activate %s (opponent was damaged)", cards_ptr[get_id(player, card)]->name);
}
while( times_damaged[p] && has_mana_multi(player, cless, black, blue, green, red, white) ){
// This is where things fall apart...
int choice = DIALOG(player, card, event, DLG_RANDOM, DLG_NO_STORAGE, DLG_NO_CANCEL,
buffer, 1, p != player ? 10 : 1,
"Decline", 5);
// .. because we always get a 0.
if( choice == 1 ){
charge_mana_multi(player, cless, black, blue, green, red, white);
if( spell_fizzled != 1 ){
SET_BYTE1(instance->info_slot) = p;
call_card_function(player, card, EVENT_DAMAGE_TRIGGER_ABILITY);
instance->info_slot = 0;
}
}
times_damaged[p]--;
times_damaged[p] = %d", times_damaged[p])
}
}
};
);
}
}
---
Argivian Archaeologist in the Library of Leng studying the Spells of the Ancients
Argivian Archaeologist in the Library of Leng studying the Spells of the Ancients
Re: [comfirmed]Treva, the Renewer 's ability doesn't trigger
by Aswan jaguar » 13 Aug 2020, 16:26
That's not a good idea because this is a trigger after all not an activation event and you will cause other issues. I have fixed other such bugs with bad use of dialoq(), unfortunately I hadn't found the cause when trying to fix this bug about a year ago but later. And I tried a lot to fix this.FastEddie wrote:I could overwrite the event with EVENT_ACTIVATE or EVENT_RESOLVE_ACTIVATION, say, to force a dialog but I don't know whether this is a good solution
I don't know why Korath did it only for those but probably had a good reason for it (maybe because it couldn't be used without causing issues both with events and with triggers). If you see how much he did for dialog_impl() he certainlyFastEddie wrote:why dialog_impl has this restrictions in the first place
programmed it with many functions and features a couple of them I believe they are not even used and others I don't know how to use them correctly. We are certainly don't use it fully. That's why I think it wasn't because he was bored or something.
In my opinion we should just replace dialoq() with the inferior do_dialoq() which works though in all cases and triggers like I did for others.
---
Trying to squash some bugs and playtesting.
Trying to squash some bugs and playtesting.
-
Aswan jaguar - Super Tester Elite
- Posts: 8078
- Joined: 13 May 2010, 12:17
- Has thanked: 730 times
- Been thanked: 458 times
Re: [comfirmed]Treva, the Renewer 's ability doesn't trigger
by drool66 » 13 Aug 2020, 16:55
Good call, FastEddie.
Fixed in a645e15d7 ("Fix Invasion Dragons' trigger dialog", 2020-08-14) by switching to do_dialog() which has fewer options but is more flexible in when it can be used. Also got rid of the card name in dialog so the options fit better on screen. Looks like this if you're interested:
Fixed in a645e15d7 ("Fix Invasion Dragons' trigger dialog", 2020-08-14) by switching to do_dialog() which has fewer options but is more flexible in when it can be used. Also got rid of the card name in dialog so the options fit better on screen. Looks like this if you're interested:
- Code: Select all
static void invasion_dragon(int player, int card, event_t event, int cless, int black, int blue, int green, int red, int white){
if (damage_dealt_by_me(player, card, event, DDBM_MUST_DAMAGE_PLAYER | DDBM_TRACE_DAMAGED_PLAYERS | DDBM_MUST_BE_COMBAT_DAMAGE))
{
card_instance_t* instance = get_card_instance(player, card);
int times_damaged[2] = { BYTE0(instance->targets[1].player), BYTE1(instance->targets[1].player) };
instance->targets[1].player = 0;
APNAP(p,{
if( times_damaged[p] ){
char buffer[100];
if( p == player ){
scnprintf(buffer, 100, " Activate (you were damaged)\n Cancel");
}
else{
scnprintf(buffer, 100, " Activate (opponent was damaged)\n Cancel");
}
while( times_damaged[p] && has_mana_multi(player, cless, black, blue, green, red, white) ){
int choice = do_dialog(player, player, card, -1, -1, buffer, p != player ? 0 : 1);
if( choice == 0 ){
charge_mana_multi(player, cless, black, blue, green, red, white);
if( spell_fizzled != 1 ){
SET_BYTE1(instance->info_slot) = p;
call_card_function(player, card, EVENT_DAMAGE_TRIGGER_ABILITY);
instance->info_slot = 0;
}
}
times_damaged[p]--;
}
}
};
);
}
}
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: [fixed]Treva, the Renewer 's ability doesn't trigger
by FastEddie » 14 Aug 2020, 05:47
Thank you guys. Looking at dialog_impl I had the impression that there is much more than meets the eye, so I didn't even dare to bother with it. Someone (Korath as I know now) spend a lot of time and effort and probably for a very good reason.
It took several iterations. I plugged everything in the code below for ease of explanation.
First, you need to inlcude the header. I do this in manalink.h as it is included everywhere. Mine looks like this:
bash: ./Magic.exe 2>Treva.log
cmd: Magic.exe 2>Treva.log
Once I attacked with Treva, the Renewer I ended the program to fully write the log file and have a look. The function is called several (30 or so) times, so I just look at the last entries. Usually the time stamp of those is a bit (a second or so) later. This went as follows.
#1 logging:
To see whether the function is entered, the player is right and the damage is assigned correctly. My first suspicion was that there is an issue with times_damaged and Trygon Predator works whereas it shouldn't.
#1 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:472 | Damage dealt by me, player = 0
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:473 | times_damaged = {0, 1}
#2 logging:
Seems to be ok. Let's see whether this squares with the damage assignment.
#2 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | Opponent was damaged
#3 logging:
Works again. So maybe we don't enter the while loop. I don't know what has_mana_multi does precisely, maybe this causes an issue. I log times_damaged for good measure so that I don't miss anything (just because you're paranoid...).
#3 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | times damaged[p] = 1
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | has_mana_multi(player, cless, black, blue, green, red, white) = 1
#4 logging:
Both 1, so we enter the while loop. If everything works as I think we should be inside only once. Let's check this.
#4 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | Entering while
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | Leaving while, times_damaged[p] = 0
#5 logging:
As expected. Good. It is likely that choice != 1. Let's see.
#5 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | choice = 0
#6 logging:
BINGO! Looking up DIALOG and dialog_impl tells me that only certain events work. No idea what event is used when calling DIALOG but obviously not the right one. Let's see which one it is.
#6 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | event = 126
Coda:
126 is 0x7E (I use the windows calculator for the conversion). Looking in defs.h I find the event code in line 230, it is called EVENT_RESOLVE_TRIGGER (which makes intuitive sense).
NB: I didn't use the debugger as I learned in the meanwhile that these functions are called several times. Always continuing the exection while running the risk to miss the proper call is cumbersome and errorprone, so this is not the preferred approach in a case like this.
Edit. I just saw that you could make your life easier during the last step with something like this, printing the event id in hex:
I know what you mean... unfortunately, this kind of bug can't be tackled with code inspection as the code looks (and mostly is) ok. Since this is a nice use case for the logger I outlined below what I did and why so you can try it at home if you are interested (will also add this to the documentation).And I tried a lot to fix this.
It took several iterations. I plugged everything in the code below for ease of explanation.
First, you need to inlcude the header. I do this in manalink.h as it is included everywhere. Mine looks like this:
- Code: Select all
#include "defs.h"
// DEBUG
#include "macrologger.h"
#define LOG_LEVEL DEBUG_LEVEL
#ifdef __cplusplus
extern "C" {
#endif
- Code: Select all
static void invasion_dragon(int player, int card, event_t event, int cless, int black, int blue, int green, int red, int white){
if (damage_dealt_by_me(player, card, event, DDBM_MUST_DAMAGE_PLAYER | DDBM_TRACE_DAMAGED_PLAYERS | DDBM_MUST_BE_COMBAT_DAMAGE))
{
card_instance_t* instance = get_card_instance(player, card);
int times_damaged[2] = { BYTE0(instance->targets[1].player), BYTE1(instance->targets[1].player) };
instance->targets[1].player = 0;
// #1
LOG_DEBUG("Damage dealt by me, player = %d", player);
LOG_DEBUG("times_damaged = {%d, %d}", times_damaged[0], times_damaged[1]);
APNAP(p,{
if( times_damaged[p] ){
char buffer[100];
if( p == player ){
scnprintf(buffer, 100, "Activate %s (you were damaged)", cards_ptr[get_id(player, card)]->name);
}
else{
scnprintf(buffer, 100, "Activate %s (opponent was damaged)", cards_ptr[get_id(player, card)]->name);
// #2
LOG_DEBUG("Opponent was damaged");
}
// #3
LOG_DEBUG("times damaged[p] = %d", times_damaged[p]);
LOG_DEBUG("has_mana_multi(player, cless, black, blue, green, red, white) = %d", has_mana_multi(player, cless, black, blue, green, red, white));
while( times_damaged[p] && has_mana_multi(player, cless, black, blue, green, red, white) ){
// #4
LOG_DEBUG("Entering while");
// #6
LOG_DEBUG("event = %d", event);
int choice = DIALOG(player, card, event, DLG_RANDOM, DLG_NO_STORAGE, DLG_NO_CANCEL,
buffer, 1, p != player ? 10 : 1,
"Decline", 5);
// #5
LOG_DEBUG("choice = %d", choice);
if( choice == 1 ){
charge_mana_multi(player, cless, black, blue, green, red, white);
if( spell_fizzled != 1 ){
SET_BYTE1(instance->info_slot) = p;
call_card_function(player, card, EVENT_DAMAGE_TRIGGER_ABILITY);
instance->info_slot = 0;
}
}
times_damaged[p]--;
// #4
LOG_DEBUG("Leaving while, times_damaged[p] = %d", times_damaged[p]);
}
}
};
);
}
}
bash: ./Magic.exe 2>Treva.log
cmd: Magic.exe 2>Treva.log
Once I attacked with Treva, the Renewer I ended the program to fully write the log file and have a look. The function is called several (30 or so) times, so I just look at the last entries. Usually the time stamp of those is a bit (a second or so) later. This went as follows.
#1 logging:
To see whether the function is entered, the player is right and the damage is assigned correctly. My first suspicion was that there is an issue with times_damaged and Trygon Predator works whereas it shouldn't.
#1 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:472 | Damage dealt by me, player = 0
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:473 | times_damaged = {0, 1}
#2 logging:
Seems to be ok. Let's see whether this squares with the damage assignment.
#2 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | Opponent was damaged
#3 logging:
Works again. So maybe we don't enter the while loop. I don't know what has_mana_multi does precisely, maybe this causes an issue. I log times_damaged for good measure so that I don't miss anything (just because you're paranoid...).
#3 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | times damaged[p] = 1
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | has_mana_multi(player, cless, black, blue, green, red, white) = 1
#4 logging:
Both 1, so we enter the while loop. If everything works as I think we should be inside only once. Let's check this.
#4 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | Entering while
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | Leaving while, times_damaged[p] = 0
#5 logging:
As expected. Good. It is likely that choice != 1. Let's see.
#5 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | choice = 0
#6 logging:
BINGO! Looking up DIALOG and dialog_impl tells me that only certain events work. No idea what event is used when calling DIALOG but obviously not the right one. Let's see which one it is.
#6 output:
2020-08-14 07:23:00 | DEBUG | invasion.c | invasion_dragon:513 | event = 126
Coda:
126 is 0x7E (I use the windows calculator for the conversion). Looking in defs.h I find the event code in line 230, it is called EVENT_RESOLVE_TRIGGER (which makes intuitive sense).
NB: I didn't use the debugger as I learned in the meanwhile that these functions are called several times. Always continuing the exection while running the risk to miss the proper call is cumbersome and errorprone, so this is not the preferred approach in a case like this.
Edit. I just saw that you could make your life easier during the last step with something like this, printing the event id in hex:
- Code: Select all
LOG_DEBUG("event = %04x", event);
---
Argivian Archaeologist in the Library of Leng studying the Spells of the Ancients
Argivian Archaeologist in the Library of Leng studying the Spells of the Ancients
Re: [comfirmed]Treva, the Renewer 's ability doesn't trigger
by Korath » 19 Aug 2020, 23:55
DIALOG() was aimed at the extremely-common case (in particular, all cards with more than one activated ability) of quickly checking whether any choices are legal, making a decision at announcement, and referencing the choice at resolution. Previously this had been implemented by cut, paste, and pray: if you look through old bug reports, you'll find literally thousands of cases where choices don't match up, or choices made for one mode impact the behavior of later activations.Aswan jaguar wrote:That's not a good idea because this is a trigger after all not an activation event and you will cause other issues. I have fixed other such bugs with bad use of dialoq(), unfortunately I hadn't found the cause when trying to fix this bug about a year ago but later. And I tried a lot to fix this.FastEddie wrote:I could overwrite the event with EVENT_ACTIVATE or EVENT_RESOLVE_ACTIVATION, say, to force a dialog but I don't know whether this is a good solutionI don't know why Korath did it only for those but probably had a good reason for it (maybe because it couldn't be used without causing issues both with events and with triggers). If you see how much he did for dialog_impl() he certainlyFastEddie wrote:why dialog_impl has this restrictions in the first place
programmed it with many functions and features a couple of them I believe they are not even used and others I don't know how to use them correctly. We are certainly don't use it fully. That's why I think it wasn't because he was bored or something.
In my opinion we should just replace dialoq() with the inferior do_dialoq() which works though in all cases and triggers like I did for others.
Using DIALOG() as a do_dialog()-with-extra-features is ok, but you have to tell it you're doing so. This is because the common case requires storage space somewhere to, at resolution, remember the choice made at activation. This is typically in card_instance_t::info_slot, though there's options to move it elsewhere. It's deliberate that it fails quickly and obviously (by doing nothing) if you give it a non-supported event, so that you know you're doing something wrong. Also, the initial intent was that you could just call DIALOG() at the start of a card function without checking event first, but that turned out to be impractical.
If you want to use it in such a case, pass DLG_NO_STORAGE and event=EVENT_CAST_SPELL (or any of the other six supported events) - this both prevents overwriting info_slot, which this card also uses for something else, and setting spell_fizzled if "Cancel" is chosen, which breaks trigger handling.
-
Korath - DEVELOPER
- Posts: 3707
- Joined: 02 Jun 2013, 05:57
- Has thanked: 496 times
- Been thanked: 1106 times
7 posts
• Page 1 of 1
Who is online
Users browsing this forum: No registered users and 56 guests