/amethyst

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

Add support for event handling #67

Merged
merged 10 commits into from Jul 18, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@nchashch @kvark @ebkalderon @Emilgardis
@nchashch
Member

nchashch commented Jul 16, 2016

Changes:

  • Add Broadcaster struct which allows publishing specs entities. An instance of Broadcaster is owned by Context.
  • Add EngineEvent component. At the begining of every Application::advance_frame() events of this type are polled from the Context and published using Broadcaster.
    Note: EngineEvent is a wraper around glutin::Event. And amethyst_context::event module reexports all event structs and enums from glutin.
  • Add Context::poll_engine_events() method. It polls platform specific events, translates them to EngneEvents and returns them.
  • Bump version of specs used by amethyst_ecs to 0.7.0
  • Reexport all specs structs, enums and traits in amethyst_ecs.
  • Add VideoContext::Null backend.

It should close #46

nchashch added some commits Jul 10, 2016

@nchashch
Add support for event handling
c2e825f
@nchashch
Add support for user defined events 
* Add Broadcaster struct to amethyst_context
* Broadcaster owns a specs::World
* Events are published by creating new entities in this World
* User can add custom event types by registering new components
* Context owns a Broadcaster, to make it globally available

* Bump specs version used in amethyst_ecs
* Export all specs structs, enums, traits in amethyst_ecs

* Make changes in amethyst_engine to add event handling
e04fbc6
@nchashch
Remove unnecessary mut in some Broadcaster methods
b50a72a
@nchashch
Update ecs/examples/usage.rs example
fed10c5
@nchashch
Update examples/hello_world.rs example 
* Add VideoContext::Null variant
* Make Null the default video context
9c9fc49
@nchashch
Add comments, refactor Broadcaster 
* Move EngineEvent struct definition and glutin reexport from
  broadcaster.rs to event.rs

* Register EngineEvent component with Broadcaster in Context::new()
  instead of Broadcaster::new
54b45cd
@nchashch
Don't return an Option in Context::new() 
* Return Context instead of Option<Context> in Context::new()
* If display_config.backend value is invalid set context.video_context
  to VideoContext::Null
46ed92c
@nchashch
Remove unnecessary specs dependencies
08a82bb
@nchashch
Fix doc comment for Context::poll_engine_events()
d2de7cb
@nchashch
Add Broadcaster example, more doc comments
5ea1218
src/context/src/broadcaster.rs
+ /// Return a vector containing clones of all published entities
+ pub fn poll(&self) -> Vec<Entity> {
+ let entities = self.world.entities();
+ let _entities: Vec<Entity> = entities.iter().map(|e| e.clone()).collect();

This comment has been minimized.

Show comment
Hide comment
@kvark

kvark Jul 18, 2016

Member

should probably keep that vector owned, re-use it, and return &[Entity] instead, to avoid re-allocations
@kvark

kvark Jul 18, 2016

Member

should probably keep that vector owned, re-use it, and return &[Entity] instead, to avoid re-allocations
@kvark

This comment has been minimized.

Show comment
Hide comment
@kvark

kvark Jul 18, 2016

Member

Looks good to me. @ebkalderon ?
Member

kvark commented Jul 18, 2016

Looks good to me. @ebkalderon ?
@ebkalderon

This comment has been minimized.

Show comment
Hide comment
@ebkalderon

ebkalderon Jul 18, 2016

Member

Solid work, @nchashch! Thank you. I think this can be merged.
Member

ebkalderon commented Jul 18, 2016

Solid work, @nchashch! Thank you. I think this can be merged.

@ebkalderon ebkalderon merged commit cb367b5 into amethyst:develop Jul 18, 2016
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
src/context/src/lib.rs
+ #[cfg(windows)]
+ VideoContext::Direct3D { } => {
+ // stub
+ let event = EngineEvent::new(Event::Closed);

This comment has been minimized.

Show comment
Hide comment
@Emilgardis

Emilgardis Jul 18, 2016

Contributor

Event is not defined. New to amethyst, but I imagine it should be glutin::Event. How did travis not pick this up?
@Emilgardis

Emilgardis Jul 18, 2016 •
edited
Edited 1 time
  • @Emilgardis Emilgardis edited Jul 18, 2016 (most recent)

Contributor

Event is not defined. New to amethyst, but I imagine it should be glutin::Event. How did travis not pick this up?

This comment has been minimized.

Show comment
Hide comment
@nchashch

nchashch Jul 18, 2016

Member

It is guarded with #[cfg(windows)], so that is probably why it is accepted by Travis, and why I missed it (I built it only on linux).
@nchashch

nchashch Jul 18, 2016 •
edited
Edited 1 time
  • @nchashch nchashch edited Jul 18, 2016 (most recent)

Member

It is guarded with #[cfg(windows)], so that is probably why it is accepted by Travis, and why I missed it (I built it only on linux).

Emilgardis added a commit to Emilgardis/amethyst that referenced this pull request Jul 18, 2016

@Emilgardis
Fixes fail to resolve `Event`. 
Travis did not pick this up in #67. This PR can be rejected and instead be rebased with e04fbc6
d3678e5
@Emilgardis

This comment has been minimized.

Show comment
Hide comment
@Emilgardis

Emilgardis Jul 18, 2016

Contributor

Used githubs provided web-interface for making pull-requests, bad mistake. Anyway, e04fbc6 should be fixed by either appending glutin:: to Event::Closed or making glutin::Event in scope via use glutin::Event.
Contributor

Emilgardis commented Jul 18, 2016 •
edited
Edited 1 time
  • @Emilgardis Emilgardis edited Jul 18, 2016 (most recent)

Used githubs provided web-interface for making pull-requests, bad mistake. Anyway, e04fbc6 should be fixed by either appending glutin:: to Event::Closed or making glutin::Event in scope via use glutin::Event.

@Emilgardis Emilgardis referenced this pull request Jul 18, 2016

Closed

Patch e04fbc6 #68

@nchashch

This comment has been minimized.

Show comment
Hide comment
@nchashch

nchashch Jul 18, 2016

Member

It would be better to import event::Event instead of glutin::Event. Because glutin::Event is reexported in amethyst_context::event module, also extern crate glutin; in amethyst_context module is unnecessary.
Member

nchashch commented Jul 18, 2016

It would be better to import event::Event instead of glutin::Event. Because glutin::Event is reexported in amethyst_context::event module, also extern crate glutin; in amethyst_context module is unnecessary.

Emilgardis added a commit to Emilgardis/amethyst that referenced this pull request Jul 19, 2016

@Emilgardis
Updated code to match #67 functionality. 
Also mentions rustup instead of multirust.
b963d8c

Emilgardis added a commit to Emilgardis/amethyst that referenced this pull request Jul 19, 2016

@Emilgardis
Updated code to match #67 functionality. 
Also mentions rustup instead of multirust.
a01a98f

@Emilgardis Emilgardis referenced this pull request Jul 19, 2016

Closed

Broadcaster is lacking features #74

Emilgardis added a commit to Emilgardis/amethyst that referenced this pull request Jul 19, 2016

@Emilgardis
Exposing `World`. 
Exposing it makes it possible to add Systems, known as Processors in
`amethyst::ecs`. This also exposes every native function.

A problem with this approach is that it is not optimal, simply using World
instead of Broadcast in `Context` is also not optimal. See amethyst/amethyst#67 and amethyst/amethyst#74.
e0222fa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment