Emitting an event with a value of 0 produces a GameEvent instead

[mattjennings]

When emitting a custom event on an object, if the value is 0 it gets overwritten by a GameEvent due to this line:

Excalibur/src/engine/EventDispatcher.ts

Lines 36 to 38 in 730d621

	if (!event) {
	event = new GameEvent();
	}

Steps to Reproduce

Emit an event with a value of 0:

actor.emit('something', 0)

actor.on('something', (value) => {
    console.log(value) // GameEvent
})

actor.emit('something', 1)

actor.on('something', (value) => {
    console.log(value) // 1
})

Expected Result

Value to be persisted

Actual Result

Value gets overwriten with GameEvent

Environment

• Excalibur versions: 0.27

Current Workaround

Create a custom GameEvent instance and emit that instead. Perhaps that is the intention, that every event should be an instance of GameEvent? If so, should emit throw an error if it receives something other than undefined or a GameEvent?

[eonarheim]

@mattjennings Thanks for the bug!

My spidey sense says this is likely bad falsey checking and providing a default GameEvent

[mattjennings]

@eonarheim yup that's what's happening here. I started working on a PR to fix this by only forcing to GameEvent if null or undefined - however I noticed the type definition for emit on EventDispatcher is:

public emit(eventName: string, event: GameEvent<T>) {}

while on Class it's:

public emit(eventName: string, eventObject: any) {}

so while I can update EventDispatcher to preserve non-GameEvents, it goes against the expected type. Should Class be updated to take GameEvent? If so, what should emit do when it receives something else?

[eonarheim]

Awesome, thanks for picking this up!

My intention is to remove the GameEvent<T> requirement eventually, it's been artificially constricting (requiring people to build custom events for simple things) and hasn't been very useful in general...

I'd support widening the type in the PR to be looser on EventDispatcher.emit(), maybe this?

// This might need some additional thought if it winds up causing typing issues elsewhere...
public emit(eventName: string, event: GameEvent<T> | any) {}

Otherwise I'm okay with a tactical change to just address the bad falsey check, and we can continue thinking/discussing the types on EventDispatcher

[mattjennings]

Good to know, I agree that it is a bit cumbersom to have to make custom events for simple values.

Doing GameEvent<T> | any effectively just casts it as any, unfortunately:

I will just address the falsey check for now then!