Implement Damage Batch for Permanent event #11841

jimga150 · 2024-02-23T19:17:49Z

Took a shot at this. I don't know where/what tests to write to test this feature since it looks like DamagedBatchForOnePlayerEvent doesn't have other tests about it as far as i can tell. What would a test suite for this look like? Is there an example of a test checking that the right event(s) fired in the right ways i can cheat off of?

Also, before that, let me know if this implementation doesn't look right.

…parate targets

JayDi85 · 2024-02-23T19:37:12Z

I don't see any reason for that event. Why you need it?

jimga150 · 2024-02-24T00:31:11Z

#11833

xenohedron · 2024-02-24T07:19:24Z

Re: the need for this event:
Donna Noble ruling:

If Donna is paired with another creature and they are both dealt damage at the same time, the second ability triggers twice.

Therefore, existing DAMAGED_BATCH_FOR_PERMANENTS won't work because there's only one event so it can't cause two triggers.

Enrage ruling:

If multiple sources deal damage to a creature with an enrage ability at the same time, most likely because multiple creatures blocked that creature, the enrage ability triggers only once. (2018-01-19)

Therefore, DAMAGED_PERMANENT can't be used directly. A batch seems like the use case for this sort of thing and how existing Enrage abilities are correctly implemented.

I suggest implementing Donna Noble on this branch and test cases as follows:

Check Donna dealt damage from two sources simultaneously = one trigger with correct amount
Check paired creature dealt damage from two sources simultaneously = one trigger with correct amount
Check Donna dealt damage twice but not simultaneously (e.g. double strike) = two triggers with correct amounts
Check Donna and paired creature simultaneously dealt damage by one source = two triggers with correct amount
Check Donna and paired creature each simultaneously dealt damage by two different sources (all in one combat) = two triggers with correct amounts.

Within each test case, make the simultaneous amounts unique where possible so correct results are clear. Set strict choose mode so order trigger choice catches correct behavior.

jimga150 · 2024-03-13T05:25:06Z

It looks like my use of CardUtil.getEventTargets(event).contains(damagedEvent.getTargetId()) (line 850, GameState.java) is catching any event that happens to contain the interested target, and then sums up the total damage dealt in that event, regardless of whether or not part of that damage was to something else.

It looks like the per-player batch is able to get around this by calling getPlayerId(), since it looks like it assumes that the event wont have multiple targets(?) whereas target IDs in DamagedBatchEvent are meant to hold any number of targets that need to be addressed as a set. Is DamagedBatchForOnePlayerEvent implemented incorrectly or is there something special that needs to be done to allow DamagedBatchForOnePermanentEvent to pick out the right damages?

Test results (play testing, working to get a proper test written):

Check Donna dealt damage from two sources simultaneously = one trigger with correct amount
-- It does trigger once, but it wraps in damage that donna dealt to other permanents in that same event. Donna attacking, blocked by a 3/4 and a 1/4 results in one trigger for 6 damage, not 4. I think the extra 2 came from donna dealing 2 to the 1/4.
Check paired creature dealt damage from two sources simultaneously = one trigger with correct amount
-- Same result as above. at least its consistent
Check Donna dealt damage twice but not simultaneously (e.g. double strike) = two triggers with correct amounts
-- double strike tested. First combat damage functions correctly, second combat damage triggers with the sum of what donna dealt and received, as before.
Check Donna and paired creature simultaneously dealt damage by one source = two triggers with correct amount
-- Actually causes one trigger with the sum of not just the damage dealt to donna and the paired card, but also what was dealt to other non-bonded creatures.
Check Donna and paired creature each simultaneously dealt damage by two different sources (all in one combat) = two triggers with correct amounts.
-- all damage dealt by all creatures in battle (donna and SB creature blocked by 2 creatures each) was summed up and dealt to opponent

Mage/src/main/java/mage/game/GameState.java

xenohedron · 2024-03-14T00:52:12Z

I'm not sure what's going on here with extraneous damage getting included. Needs more research and/or debugging I guess.

jimga150 · 2024-03-14T03:22:03Z

Here's a conundrum:

I added some logging to GameState and DamagedBatchEvent:

GameState::addSimultaneousDamage:

            // per permanent
            if (isPermanentDamage && event instanceof DamagedBatchForOnePermanentEvent) {
                DamagedBatchForOnePermanentEvent oldPermanentBatch = (DamagedBatchForOnePermanentEvent) event;
                if (oldPermanentBatch.getDamageClazz().isInstance(damagedEvent)
//                        && CardUtil.getEventTargets(event).toArray()[0].equals(damagedEvent.getTargetId())) {
                        && CardUtil.getEventTargets(event).contains(damagedEvent.getTargetId())) {
                    logger.info("Added damage event targeting " + damagedEvent.getTargetId());
                    logger.info("To batch event targeting " + CardUtil.getEventTargets(event).toArray()[0]);
                    logger.info("total damage on this batch: " + oldPermanentBatch.getAmount());
                    oldPermanentBatch.addEvent(damagedEvent);
                    logger.info("Batch event is targeting " + CardUtil.getEventTargets(event).toArray()[0] + " after addEvent call");
                    isPermanentBatchUsed = true;
                }
            }

...

        if (!isPermanentBatchUsed && isPermanentDamage) {
            DamagedBatchEvent event = new DamagedBatchForOnePermanentEvent(damagedEvent.getTargetId());
            event.addEvent(damagedEvent);
            addSimultaneousEvent(event, game);
            logger.info("Created batch damage event targeting " + damagedEvent.getTargetId());
            logger.info("total damage on this batch: " + event.getAmount());
        }

DamagedBatchEvent:

    @Override
    public int getAmount() {
        int ans = events
                .stream()
                .mapToInt(GameEvent::getAmount)
                .sum();
        logger.info("getAmount called, returning " + ans + " damage from " + events.size() + " events");
        return ans;
    }

Now if i run my test case, where a 3/4 and a 1/4 block Donna Noble, i get this:

INFO  2024-03-13 23:05:28,476 Created batch damage event targeting 459fc73b-3e3e-4dc2-af44-b4e5a9d76fbf                  =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,477 getAmount called, returning 3 damage from 1 events                                         =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:05:28,477 total damage on this batch: 3                                                              =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,479 Added damage event targeting 459fc73b-3e3e-4dc2-af44-b4e5a9d76fbf                          =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,479 To batch event targeting 459fc73b-3e3e-4dc2-af44-b4e5a9d76fbf                              =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,479 getAmount called, returning 4 damage from 2 events                                         =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:05:28,480 total damage on this batch: 4                                                              =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,480 Batch event is targeting 459fc73b-3e3e-4dc2-af44-b4e5a9d76fbf after addEvent call          =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,480 Added damage event targeting 74062a24-6931-4a89-ba41-ff11f9b94639                          =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,480 To batch event targeting 459fc73b-3e3e-4dc2-af44-b4e5a9d76fbf                              =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,480 getAmount called, returning 6 damage from 3 events                                         =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:05:28,480 total damage on this batch: 6                                                              =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,481 Batch event is targeting 459fc73b-3e3e-4dc2-af44-b4e5a9d76fbf after addEvent call          =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:05:28,482 test                                                                                       =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] DonnaNobleTriggeredAbility.checkTrigger 
INFO  2024-03-13 23:05:28,482 getAmount called, returning 6 damage from 3 events                                         =>[GAME 9dffaf10-1433-4d0f-8ed4-a39ec0b09bbc] DamagedBatchEvent.getAmount

So it seems like the CardUtil.getEventTargets(event).contains(damagedEvent.getTargetId()) check is matching that which it should not.

I replaced this clause with the following:
CardUtil.getEventTargets(event).toArray()[0].equals(damagedEvent.getTargetId())

Now, i get this:

INFO  2024-03-13 23:16:07,287 Created batch damage event targeting 387f6d13-ce20-4c8b-a7c4-d6460b2f9b24                  =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,287 getAmount called, returning 3 damage from 1 events                                         =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:16:07,288 total damage on this batch: 3                                                              =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,290 Added damage event targeting 387f6d13-ce20-4c8b-a7c4-d6460b2f9b24                          =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,290 To batch event targeting 387f6d13-ce20-4c8b-a7c4-d6460b2f9b24                              =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,291 getAmount called, returning 4 damage from 2 events                                         =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:16:07,291 total damage on this batch: 4                                                              =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,291 Batch event is targeting 387f6d13-ce20-4c8b-a7c4-d6460b2f9b24 after addEvent call          =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,291 Added damage event targeting e4185fff-d81d-47ee-bdb9-4610eabad09a                          =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,292 To batch event targeting e4185fff-d81d-47ee-bdb9-4610eabad09a                              =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,292 getAmount called, returning 6 damage from 3 events                                         =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:16:07,292 total damage on this batch: 6                                                              =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,292 Batch event is targeting e4185fff-d81d-47ee-bdb9-4610eabad09a after addEvent call          =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:16:07,293 test                                                                                       =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] DonnaNobleTriggeredAbility.checkTrigger 
INFO  2024-03-13 23:16:07,293 getAmount called, returning 6 damage from 3 events                                         =>[GAME aeb8fceb-f833-4f96-a487-437c30f986c1] DamagedBatchEvent.getAmount

So now instead of making it look like it added damagedEvent to a batch that it shouldnt have, it now looks like there was a batch event for the other target... already there? without having created it. Which makes even less sense because the behavior is unchanged from the first run--i still get one trigger for 6 damage.

Is something i'm calling here subtly changing values and i don't see it? the equals() function is definitely an equality checker, not assignment (sanity checking myself)

jimga150 · 2024-03-14T03:48:27Z

Also, based on the order that these prints are made in relation to addEvent, it looks like the logger is resolving inline code after its called, not when its called. does the logger have some fort of flush function? i dont see one perusing.

xenohedron · 2024-03-14T03:53:45Z

I think due to threads there may not be any guarantee on precise ordering of the logger calls.

I've read through this a few times and haven't yet come up with a plausible explanation for the behavior you've observed. Quite interesting...

JayDi85 · 2024-03-14T04:13:24Z

Xmage uses single game thread (for real game, for unit test, etc). No parallel executions. Only AI's game simulations and some rare user commands (#11460) executed in non game thread. Don't forget to filter logs by game.isSimulation(). So logs order is a real.

JayDi85 · 2024-03-14T04:17:20Z

You can insert new Throwable().printStackTrace(); in some places to see a full stack and find original code that was called.

jimga150 · 2024-03-14T04:26:11Z

~~I think the logger's threading relationship with the actual code might be causing some issues with scrutinizing the code's actual behavior. Either that, or~~ (just saw your previous comments)

Maybe somehow the version of the code generating the logs is different from that which is being used to actually collect the events. I'm rebuilding the client and server each run, not sure how that might happen but its sitting in the back of my head.

New printouts to see if there's more than one target in the batch event being added:

            // per permanent
            if (isPermanentDamage && event instanceof DamagedBatchForOnePermanentEvent) {
                DamagedBatchForOnePermanentEvent oldPermanentBatch = (DamagedBatchForOnePermanentEvent) event;
                if (oldPermanentBatch.getDamageClazz().isInstance(damagedEvent)
//                        && CardUtil.getEventTargets(event).size() == 1
//                        && CardUtil.getEventTargets(event).toArray()[0].equals(damagedEvent.getTargetId())) {
                        && CardUtil.getEventTargets(event).contains(damagedEvent.getTargetId())) {

                    logger.info("Adding " + damagedEvent.getAmount() + " damage event targeting " + damagedEvent.getTargetId());

                    logger.info("To batch event targeting the following (before addEvent):");
                    for (UUID id : CardUtil.getEventTargets(event)) logger.info(id);
                    logger.info("total damage on this batch (before addEvent): " + oldPermanentBatch.getAmount());

                    oldPermanentBatch.addEvent(damagedEvent);

                    logger.info("Batch event is targeting the following (after addEvent):");
                    for (UUID id : CardUtil.getEventTargets(event)) logger.info(id);
                    logger.info("total damage on this batch (after addEvent): " + oldPermanentBatch.getAmount());

                    isPermanentBatchUsed = true;
                }
            }

        if (!isPermanentBatchUsed && isPermanentDamage) {
            DamagedBatchEvent event = new DamagedBatchForOnePermanentEvent(damagedEvent.getTargetId());
            event.addEvent(damagedEvent);
            addSimultaneousEvent(event, game);
            logger.info("Created batch damage event with " + damagedEvent.getAmount() + " damage, targeting " + damagedEvent.getTargetId());
            logger.info("total damage on this batch: " + event.getAmount());
        }

And i'm trying a few combinations of the following three clauses:

//                        && CardUtil.getEventTargets(event).size() == 1
//                        && CardUtil.getEventTargets(event).toArray()[0].equals(damagedEvent.getTargetId())) {
                        && CardUtil.getEventTargets(event).contains(damagedEvent.getTargetId())) {

Using the contains() clause, as shown above. This results in the third damage event (always the one targeting one of the blocked creatures, not Donna) being wrapped into the one batch event meant for Donna:

(i added java syntax highlighting to these logs cause to me it makes it easier to track whats going on)

INFO  2024-03-13 23:44:20,609 Created batch damage event with 3 damage, targeting c123032c-197a-4668-89b5-b25f5e15888b   =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,610 getAmount called, returning 3 damage from 1 events                                         =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:44:20,610 total damage on this batch: 3                                                              =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,612 Adding 1 damage event targeting c123032c-197a-4668-89b5-b25f5e15888b                       =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,612 To batch event targeting the following (before addEvent):                                  =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,613 c123032c-197a-4668-89b5-b25f5e15888b                                                       =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,613 getAmount called, returning 4 damage from 2 events                                         =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:44:20,613 total damage on this batch (before addEvent): 4                                            =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,613 Batch event is targeting the following (after addEvent):                                   =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,613 c123032c-197a-4668-89b5-b25f5e15888b                                                       =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,613 getAmount called, returning 4 damage from 2 events                                         =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:44:20,613 total damage on this batch (after addEvent): 4                                             =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,614 Adding 2 damage event targeting 813bee4f-ad85-4d76-8d62-28667d071c0f                       =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,614 To batch event targeting the following (before addEvent):                                  =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,614 c123032c-197a-4668-89b5-b25f5e15888b                                                       =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,615 813bee4f-ad85-4d76-8d62-28667d071c0f                                                       =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,615 getAmount called, returning 6 damage from 3 events                                         =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:44:20,615 total damage on this batch (before addEvent): 6                                            =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,615 Batch event is targeting the following (after addEvent):                                   =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,615 c123032c-197a-4668-89b5-b25f5e15888b                                                       =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,615 813bee4f-ad85-4d76-8d62-28667d071c0f                                                       =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,615 getAmount called, returning 6 damage from 3 events                                         =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] DamagedBatchEvent.getAmount 
INFO  2024-03-13 23:44:20,616 total damage on this batch (after addEvent): 6                                             =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] GameState.addSimultaneousDamage 
INFO  2024-03-13 23:44:20,617 test                                                                                       =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] DonnaNobleTriggeredAbility.checkTrigger 
INFO  2024-03-13 23:44:20,617 getAmount called, returning 6 damage from 3 events                                         =>[GAME 7a1a40d3-5610-48e8-a9d1-5ef8eab60b07] DamagedBatchEvent.getAmount

Using just the toArray()[0].equals() clause:

                if (oldPermanentBatch.getDamageClazz().isInstance(damagedEvent)
//                        && CardUtil.getEventTargets(event).size() == 1
                        && CardUtil.getEventTargets(event).toArray()[0].equals(damagedEvent.getTargetId())) {
//                        && CardUtil.getEventTargets(event).contains(damagedEvent.getTargetId())) {

This results in the same logs:

INFO  2024-03-14 00:12:08,114 Created batch damage event with 3 damage, targeting c56477b9-aecb-47f5-963e-22d63ddd80b9   =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,115 getAmount called, returning 3 damage from 1 events                                         =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] DamagedBatchEvent.getAmount 
INFO  2024-03-14 00:12:08,115 total damage on this batch: 3                                                              =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,117 Adding 1 damage event targeting c56477b9-aecb-47f5-963e-22d63ddd80b9                       =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,117 To batch event targeting the following (before addEvent):                                  =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,117 c56477b9-aecb-47f5-963e-22d63ddd80b9                                                       =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,118 getAmount called, returning 4 damage from 2 events                                         =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] DamagedBatchEvent.getAmount 
INFO  2024-03-14 00:12:08,118 total damage on this batch (before addEvent): 4                                            =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,118 Batch event is targeting the following (after addEvent):                                   =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,118 c56477b9-aecb-47f5-963e-22d63ddd80b9                                                       =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,118 getAmount called, returning 4 damage from 2 events                                         =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] DamagedBatchEvent.getAmount 
INFO  2024-03-14 00:12:08,118 total damage on this batch (after addEvent): 4                                             =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,118 Adding 2 damage event targeting bba2bc93-dc7b-4b76-9613-066f4b6112c4                       =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,119 To batch event targeting the following (before addEvent):                                  =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,119 bba2bc93-dc7b-4b76-9613-066f4b6112c4                                                       =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,119 c56477b9-aecb-47f5-963e-22d63ddd80b9                                                       =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,119 getAmount called, returning 6 damage from 3 events                                         =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] DamagedBatchEvent.getAmount 
INFO  2024-03-14 00:12:08,119 total damage on this batch (before addEvent): 6                                            =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,119 Batch event is targeting the following (after addEvent):                                   =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,119 bba2bc93-dc7b-4b76-9613-066f4b6112c4                                                       =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,119 c56477b9-aecb-47f5-963e-22d63ddd80b9                                                       =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,120 getAmount called, returning 6 damage from 3 events                                         =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] DamagedBatchEvent.getAmount 
INFO  2024-03-14 00:12:08,120 total damage on this batch (after addEvent): 6                                             =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:12:08,121 test                                                                                       =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] DonnaNobleTriggeredAbility.checkTrigger 
INFO  2024-03-14 00:12:08,121 getAmount called, returning 6 damage from 3 events                                         =>[GAME f424f4b9-eab3-4670-8962-5541e5887ae9] DamagedBatchEvent.getAmount

using the toArray()[0].equals() clause AND the size() == 1 clause:

                if (oldPermanentBatch.getDamageClazz().isInstance(damagedEvent)
                        && CardUtil.getEventTargets(event).size() == 1
                        && CardUtil.getEventTargets(event).toArray()[0].equals(damagedEvent.getTargetId())) {
//                        && CardUtil.getEventTargets(event).contains(damagedEvent.getTargetId())) {

This causes what looks, on paper (at least before you see the last line) to be the correct behavior! However the actual behavior is then unchanged:

INFO  2024-03-14 00:05:50,520 Created batch damage event with 3 damage, targeting da83c207-fa25-4218-9f05-6700e38bd3d2   =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,521 getAmount called, returning 3 damage from 1 events                                         =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] DamagedBatchEvent.getAmount 
INFO  2024-03-14 00:05:50,521 total damage on this batch: 3                                                              =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,524 Adding 1 damage event targeting da83c207-fa25-4218-9f05-6700e38bd3d2                       =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,524 To batch event targeting the following (before addEvent):                                  =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,524 da83c207-fa25-4218-9f05-6700e38bd3d2                                                       =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,525 getAmount called, returning 4 damage from 2 events                                         =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] DamagedBatchEvent.getAmount 
INFO  2024-03-14 00:05:50,525 total damage on this batch (before addEvent): 4                                            =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,525 Batch event is targeting the following (after addEvent):                                   =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,525 da83c207-fa25-4218-9f05-6700e38bd3d2                                                       =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,525 getAmount called, returning 4 damage from 2 events                                         =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] DamagedBatchEvent.getAmount 
INFO  2024-03-14 00:05:50,525 total damage on this batch (after addEvent): 4                                             =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,526 Created batch damage event with 2 damage, targeting 67cc2850-c785-4feb-a0aa-a36aa9c82654   =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,526 getAmount called, returning 2 damage from 1 events                                         =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] DamagedBatchEvent.getAmount 
INFO  2024-03-14 00:05:50,526 total damage on this batch: 2                                                              =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] GameState.addSimultaneousDamage 
INFO  2024-03-14 00:05:50,527 test                                                                                       =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] DonnaNobleTriggeredAbility.checkTrigger 
INFO  2024-03-14 00:05:50,528 getAmount called, returning 6 damage from 3 events                                         =>[GAME fc30d416-9809-4753-a0a2-c7c8e5c02fc7] DamagedBatchEvent.getAmount

jimga150 · 2024-03-14T04:33:49Z

You can insert new Throwable().printStackTrace(); in some places to see a full stack and find original code that was called.

Tried this, it provided what i would expect normally--the correct line number in the correct file, under a mountain of calls within XMage's call stack.

jimga150 · 2024-03-14T19:37:25Z

I found the issue.

This snippet adding individual damage events to existing damage batches should be happening only if that damage event hasn't already been added to the current event being iterated over:

            // per damage type
            if ((event instanceof DamagedBatchEvent)
                    && ((DamagedBatchEvent) event).getDamageClazz().isInstance(damagedEvent)) {
                ((DamagedBatchEvent) event).addEvent(damagedEvent);
                isDamageBatchUsed = true;
            }

This explains why the behavior is unchanged no matter what logic i use, and why the printouts seem to know ahead of the addEvent call that the event is added--because this snippet had beaten my code to the punch every time. I'm going to reorder this in an if/elseif/else case to fix this in another PR and write a full test for donna noble in this one.

github-actions · 2024-03-14T19:37:37Z

Unable to retrieve information for "DamagedBatchEvent) event).getDamageClazz().isInstance(damagedEvent"

…amagedBatchForOnePlayerEvent instances when they shouldnt

…if tree to match new logic

# Conflicts: # Mage/src/main/java/mage/game/GameState.java

jimga150 · 2024-03-16T18:50:20Z

Should be ready to merge now. Let me know if i should move any tests to a different path

Mage/src/main/java/mage/game/GameState.java

Mage.Tests/src/test/java/org/mage/test/game/events/DamagedBatchTests.java

Mage.Sets/src/mage/cards/d/DonnaNoble.java

jimga150 added 5 commits February 21, 2024 23:28

Initial Commit

2fa2218

Changed trigger to DAMAGED_BATCH_FOR_PERMANENTS, check for need of se…

2fd4f47

…parate targets

fix short circuit operator

342d2f5

simplify control path in paired damage trigger

8339d82

Initial commit, missing tests

a262ff3

github-actions bot added the engine label Feb 23, 2024

jimga150 added 2 commits February 23, 2024 19:54

use CardUtil.getEventTargets

df705b6

Merge branch 'master' into damaged-batch-for-permanent

df91f27

jimga150 added 4 commits February 26, 2024 14:41

Merge branch 'donna-noble' into damaged-batch-for-permanent

b427598

Implement Donna Noble using DamagedBatchForOnePermanentEvent

31a7bb7

fix double-effect bug

e9d9cd5

remove unnecessary custom effect

9d12024

github-actions bot added the cards label Mar 13, 2024

xenohedron reviewed Mar 14, 2024

View reviewed changes

Mage/src/main/java/mage/game/GameState.java Show resolved Hide resolved

Fix addSimultaneousDamage to avoid adding damage events to existing D…

0012a0d

…amagedBatchForOnePlayerEvent instances when they shouldnt

jimga150 mentioned this pull request Mar 14, 2024

Fix addSimultaneousDamage to avoid adding damage events to existing DamagedBatchForOnePlayerEvent instances when they shouldn't #11943

Merged

jimga150 added 4 commits March 14, 2024 19:00

Add clarifying comment

4b44dfd

Merge branch 'fix-damage-batch-player' into damaged-batch-for-permanent

a7660d7

Incorporate batching of DAMAGED_BATCH_FOR_ONE_PERMANENT into if-else …

1d53b72

…if tree to match new logic

Add tests

f69818e

github-actions bot added the tests label Mar 16, 2024

Merge branch 'master' into damaged-batch-for-permanent

1f41864

# Conflicts: # Mage/src/main/java/mage/game/GameState.java

xenohedron approved these changes Mar 16, 2024

View reviewed changes

Mage/src/main/java/mage/game/GameState.java Outdated Show resolved Hide resolved

Mage.Tests/src/test/java/org/mage/test/game/events/DamagedBatchTests.java Outdated Show resolved Hide resolved

Mage.Sets/src/mage/cards/d/DonnaNoble.java Outdated Show resolved Hide resolved

jimga150 added 3 commits March 16, 2024 22:17

make ability inline

923ff16

Move DamageBatchTests

761ab1c

Change batch events to take first event in constructor

2fafee9

xenohedron merged commit 67286aa into magefree:master Mar 17, 2024
2 checks passed

jimga150 mentioned this pull request Mar 18, 2024

Fix Wrathful Raptor's ability trigger #11961

Merged

xenohedron mentioned this pull request Mar 23, 2024

Rework BatchGameEvent and subclasses to deduplicate code and logic #11985

Closed

jimga150 mentioned this pull request Mar 24, 2024

Work outline - Fixing enrage and similar triggered abilities #11992

Closed

jimga150 mentioned this pull request Apr 1, 2024

Quartzwood Crasher creates tokens too big when attacking multiple players #12013

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Damage Batch for Permanent event #11841

Implement Damage Batch for Permanent event #11841

jimga150 commented Feb 23, 2024

JayDi85 commented Feb 23, 2024

jimga150 commented Feb 24, 2024

xenohedron commented Feb 24, 2024

jimga150 commented Mar 13, 2024 •

edited

Loading

xenohedron commented Mar 14, 2024

jimga150 commented Mar 14, 2024

jimga150 commented Mar 14, 2024

xenohedron commented Mar 14, 2024

JayDi85 commented Mar 14, 2024 •

edited

Loading

JayDi85 commented Mar 14, 2024

jimga150 commented Mar 14, 2024 •

edited

Loading

jimga150 commented Mar 14, 2024

jimga150 commented Mar 14, 2024

github-actions bot commented Mar 14, 2024

jimga150 commented Mar 16, 2024

Implement Damage Batch for Permanent event #11841

Implement Damage Batch for Permanent event #11841

Conversation

jimga150 commented Feb 23, 2024

JayDi85 commented Feb 23, 2024

jimga150 commented Feb 24, 2024

xenohedron commented Feb 24, 2024

jimga150 commented Mar 13, 2024 • edited Loading

xenohedron commented Mar 14, 2024

jimga150 commented Mar 14, 2024

jimga150 commented Mar 14, 2024

xenohedron commented Mar 14, 2024

JayDi85 commented Mar 14, 2024 • edited Loading

JayDi85 commented Mar 14, 2024

jimga150 commented Mar 14, 2024 • edited Loading

jimga150 commented Mar 14, 2024

jimga150 commented Mar 14, 2024

github-actions bot commented Mar 14, 2024

jimga150 commented Mar 16, 2024

jimga150 commented Mar 13, 2024 •

edited

Loading

JayDi85 commented Mar 14, 2024 •

edited

Loading

jimga150 commented Mar 14, 2024 •

edited

Loading