Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fix TestEventReceiverTest by giving target entity a DummyComponent #56

Merged
merged 3 commits into from
Aug 16, 2021

Conversation

skaldarnar
Copy link
Contributor

@skaldarnar skaldarnar commented Jul 3, 2021

By working on #55 we discovered that some of the tests are broken independent of any gestalt and module management issues.

This PR fixes those tests (at least locally) by giving the target entity a DummyComponent, as is expected by DummySystem.

@skaldarnar skaldarnar added the Category: Test/QA Requests, Issues and Changes targeting tests and quality assurance label Jul 3, 2021
@keturn
Copy link
Contributor

keturn commented Jul 3, 2021

Could we add a component filter to DummySystem's @ReceiveEvent instead? Would that be better?

@skaldarnar
Copy link
Contributor Author

skaldarnar commented Jul 3, 2021

Could we add a component filter to DummySystem's @ReceiveEvent instead? Would that be better?

Done in 9e07194

I think we need both, at least I hope that the goal of sending this event is that the system reacts to it... 👀

Copy link
Member

@jdrueckert jdrueckert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
@keturn okay with you to merge?

@keturn
Copy link
Contributor

keturn commented Aug 16, 2021

oh, I was thinking more like

    @ReceiveEvent(components=DummyComponent.class)
    public void onDummyEvent(DummyEvent event, EntityRef entity /*, DummyComponent component */) 

but if we can put the component in the signature to skip the entity.getComponent call, that's handy too.

Does it do the same thing as naming the component in the annotation, or is that component argument nullable?

either way, I'm not too fussed about it, feel free to merge 👍

@keturn
Copy link
Contributor

keturn commented Aug 16, 2021

I think last time I looked at this I got really distracted by the question of what was registering that system and if there were any way to selectively avoid it.

@skaldarnar
Copy link
Contributor Author

Does it do the same thing as naming the component in the annotation, or is that component argument nullable?

@keturn afaik these arguments are not nullable and serve the same purpose as listing them in the annotation, except that you can take a shortcut for giving them a name and retrieving them from the entity.

I couldn't find any good/clear documentation on this, though (for the annotation it is described in the Javadoc). There's some information in the Entity System Tutorial as well as in the Event Interaction Tutorial.

The engine wiki has some info on Events in Detail which is not nearly as detailed as it should be 🙈 ... but another page on Processing Events has more info on it 🙃

@skaldarnar skaldarnar merged commit 8002091 into develop Aug 16, 2021
@skaldarnar skaldarnar deleted the test/fix-tests-missing-dummycomponent branch August 16, 2021 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Test/QA Requests, Issues and Changes targeting tests and quality assurance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants