carpet-discarpet icon indicating copy to clipboard operation
carpet-discarpet copied to clipboard

__on_system_message does not fire for any system message produced by scarpet

Open GunnyWaffle opened this issue 1 year ago • 7 comments

I'm not sure if this is a scarpet bug or a discarpet bug, but this feels like the correct project to report the issue to.

If you use the run() function to grant an advancement to a player, the __on_system_message event does not fire.

For example:

/script run run('advancement grant @s only minecraft:adventure/arbalistic')

Thanks again for the support!

GunnyWaffle avatar Dec 15 '23 20:12 GunnyWaffle

This appears to apply to any messages generated from a script.

/script run run('say Blahblahblah!')
/script run run('kill')
/script run modify(player(), 'kill')
/script run inventory_set(player(), 0, 1, 'elytra')

In the case of the last example, it grants the "Sky's the Limit" advancement without invoking a system message event.

YotaXP avatar Dec 15 '23 21:12 YotaXP

On my testing, this seems to work (latest version, 1.20.4)

I was using this test script:

__config() -> {'scope'->'global'};

__on_system_message(text,type) -> (
    print(player('all'), 'SYS:' + text);
);

Could it be that you were testing it with a player scoped script? In that case it makes sense that it wouldnt trigger the event for any player, since the command source is always the server.

replaceitem avatar Dec 16 '23 14:12 replaceitem

You know what, I should have specified version. That's my bad. This is happening in 1.20.2.

I will close this issue if this is resolved once we update to 1.20.4.

The scarpet apps are global scoped.

GunnyWaffle avatar Dec 16 '23 15:12 GunnyWaffle

Sorry for the long wait! I've tested this in 1.20.4 in every way that we found the issue and it does in fact work now.

I'm closing this issue as resolved.

GunnyWaffle avatar Jan 15 '24 06:01 GunnyWaffle

I have bad news. :( I pushed my code to production alongside updating to 1.20.4 and the problem returned anew.

To work around the issue, I've implemented a bit of a hack. When granting an advancement in any scarpet app, I also signal a custom event with player/advancement information in tow. Then my Discord bridge app handles the custom event and generates the arguments for the __on_system_message method to call it directly.

global_advancement_event = 'advancement_event_workaround';
handle_event(global_advancement_event, _(e) -> (
	[player_name, advancement, advancement_name, tier] = e;
	
	has_advancement = bool(entity_selector('@p[name=' + player_name + ',advancements={' + advancement + '=true}]'));
	if (has_advancement, return());
	
	if (tier == 'task',
		verbage = 'made the advancement';
	,tier == 'goal',
		verbage = 'reached the goal';
	,tier == 'challenge',
		verbage = 'completed the challenge';
	);
	
	text = str('%s has %s [%s]', player_name, verbage, advancement_name);
	__on_system_message(text, 'advancement.' + tier);
));

I'm going to scratch at this for a while to see what I can find for a repro, but I think I found a good one.

/script run schedule(20, _()->(run('advancement grant @s only minecraft:husbandry/tame_an_animal')))

GunnyWaffle avatar Jan 15 '24 08:01 GunnyWaffle

I dug a little deeper, and your snippet using schedule doesn't work because events cant trigger from inside events. Scheduled calls are also treated that way. So when the advancement is granted inside the scheduled call, it sends the system message, tries to trigger the event, but doesn't because the call is already considered inside an event due to the schedule. This could be worked around by using a task (Since this works per thread)

replaceitem avatar Jan 15 '24 14:01 replaceitem

I suppose that also explains why granting an advancement in __on_tick() doesn't work either, when I tried to setup a deferred queue of advancements to grant.

I'll have time to try task in our production code later tonight or tomorrow. Thanks for looking into this!

GunnyWaffle avatar Jan 15 '24 17:01 GunnyWaffle

Hey there! Sorry for taking so long to reply. Very long story short: My home flooded literally a few hours after I sent my previous message. Things are better now! Which means I finally got to try out your suggestion.

Using task to grant advancements works flawlessly. All the issues have been resolved. Thank you! Your explanation makes perfect sense to me.

GunnyWaffle avatar May 18 '24 14:05 GunnyWaffle