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

`mouse::get_position(ctx)?` results in `EventPump` SDL error. #283

Closed
dannyfritz opened this issue Feb 13, 2018 · 4 comments

Comments

@dannyfritz
Copy link
Contributor

commented Feb 13, 2018

I'm having trouble using mouse::get_position. Is there a certain location I can use it? Seems using it within the event loop doesn't work for me.

  fn update(&mut self, ctx: &mut Context) -> GameResult<()> {
      self.pos = mouse::get_position(ctx)?;
      Ok(())
  }

results in

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: UnknownError("an `EventPump` instance is already alive - there can only be one `EventPump` in use at a time.")', src\libcore\result.rs:906:4
stack backtrace:
   0: std::sys_common::backtrace::_print
             at C:\projects\rust\src\libstd\sys_common\backtrace.rs:91
   1: std::panicking::default_hook::{{closure}}
             at C:\projects\rust\src\libstd\panicking.rs:380
   2: std::panicking::default_hook
             at C:\projects\rust\src\libstd\panicking.rs:397
   3: std::panicking::rust_panic_with_hook
             at C:\projects\rust\src\libstd\panicking.rs:577
   4: std::panicking::begin_panic<alloc::string::String>
             at C:\projects\rust\src\libstd\panicking.rs:538
   5: std::panicking::begin_panic_fmt
             at C:\projects\rust\src\libstd\panicking.rs:522
   6: std::panicking::rust_begin_panic
             at C:\projects\rust\src\libstd\panicking.rs:498
   7: core::panicking::panic_fmt
             at C:\projects\rust\src\libcore\panicking.rs:71
   8: core::result::unwrap_failed<ggez::error::GameError>
             at C:\projects\rust\src\libcore\macros.rs:23
   9: core::result::Result<(), ggez::error::GameError>::unwrap<(),ggez::error::GameError>
             at C:\projects\rust\src\libcore\result.rs:772
  10: mouse_trail::main
             at .\src\main.rs:35
  11: panic_unwind::__rust_maybe_catch_panic
             at C:\projects\rust\src\libpanic_unwind\lib.rs:101
  12: std::rt::lang_start
             at C:\projects\rust\src\libstd\rt.rs:51
  13: main
  14: __scrt_common_main_seh
             at f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283
  15: BaseThreadInitThunk

@dannyfritz dannyfritz changed the title moues::get_position(ctx)? results in `EventPump` SDL error. `mouse::get_position(ctx)?` results in `EventPump` SDL error. Feb 13, 2018

@icefoxen

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2018

Confirmed on master and devel branches. I think I know what's going on but it's going to take some sticky lifetime stuff to fix. I suggest using the EventHandler::mouse_motion_event() callback in the mean time.

@icefoxen

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2018

It was more sticky than anticipated! This should be fixed in devel branch though, please test.

However, I don't like it, because it requires storing the mouse position from events in event::run(). So if you roll your own event loop the mouse position no longer gets updated. I do not like making this necessary because it's super easy to miss. I dislike it more because there's already a way to get mouse position, from the MouseMotion event. Adding a workaround that violates OnceAndOnlyOnce seems dirty, even if it is for the sake of API symmetry -- the alternative is to have mouse::set_position() with no matching mouse::get_position(), which doesn't seem much better.

One possible solution would be to add a hook to feed events into the Context so it can update its own internal state based on them as necessary, which would cover both this case and the resize_viewport() call that's there anyway. Basically, you would call ctx.process_events() on each event at the start of your own event loop. That's probably the right choice...

icefoxen added a commit that referenced this issue Feb 15, 2018

@dannyfritz

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2018

Thanks for the information, I'll use MouseMotion for now and look at writing an event loop at another time.

@icefoxen

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2018

Closing this, pondering API enhancements continued in #284

@icefoxen icefoxen closed this Feb 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.