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

returning Push function when screen hasExited. #1241

Merged
merged 4 commits into from Dec 21, 2017

Conversation

3 participants
@DevSDK
Contributor

DevSDK commented Dec 9, 2017

The reason is that push function work with async.
So, That can be wrong work. Because Exit function can be called before async push function done.

I inserted function escape code when this scene exited.

returning Push function when screen hasExited.
The reason is that push function work with async.
So, That can be wrong work. Because Exit function can be called before async push function done.

I inserted function escape code when this scene exited.
@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Dec 10, 2017

Member

What's the scenario here? You shouldn't be calling Push from a non-update thread, if that's what you're attempting.

Member

peppy commented Dec 10, 2017

What's the scenario here? You shouldn't be calling Push from a non-update thread, if that's what you're attempting.

@DevSDK

This comment has been minimized.

Show comment
Hide comment
@DevSDK

DevSDK Dec 10, 2017

Contributor

Well, You can try to Press enter or click to Song Select And Press ESC and select again faster (Iterate)
OSU died.
I found that the push function work with hasExit value is true.

Contributor

DevSDK commented Dec 10, 2017

Well, You can try to Press enter or click to Song Select And Press ESC and select again faster (Iterate)
OSU died.
I found that the push function work with hasExit value is true.

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Dec 10, 2017

Member

I see. Not sure this is the correct path forward though. This may be best off throwing an exception, along with blocking off the paths which are causing these issues in the first place (maybe something like overriding HandleInput to not handle input after we have been exited).

Needs a bit more investigation.

Member

peppy commented Dec 10, 2017

I see. Not sure this is the correct path forward though. This may be best off throwing an exception, along with blocking off the paths which are causing these issues in the first place (maybe something like overriding HandleInput to not handle input after we have been exited).

Needs a bit more investigation.

@DevSDK

This comment has been minimized.

Show comment
Hide comment
@DevSDK

DevSDK Dec 10, 2017

Contributor

You mean better push function to work with sync?
Or I need find other way?

Contributor

DevSDK commented Dec 10, 2017

You mean better push function to work with sync?
Or I need find other way?

@DevSDK

This comment has been minimized.

Show comment
Hide comment
@DevSDK

DevSDK Dec 10, 2017

Contributor

How Do you think about separate that exceptions hasExit == true and player ==null
I think that diffetent way exceptioms

Contributor

DevSDK commented Dec 10, 2017

How Do you think about separate that exceptions hasExit == true and player ==null
I think that diffetent way exceptioms

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Dec 18, 2017

Member

this should definitely be an exception, not just a return false

Member

peppy commented Dec 18, 2017

this should definitely be an exception, not just a return false

@DevSDK

This comment has been minimized.

Show comment
Hide comment
@DevSDK

DevSDK Dec 18, 2017

Contributor

Ummm, Your mean implement exception handler batter then return false?
Well, can i try this?

Contributor

DevSDK commented Dec 18, 2017

Ummm, Your mean implement exception handler batter then return false?
Well, can i try this?

@smoogipoo

This comment has been minimized.

Show comment
Hide comment
@smoogipoo

smoogipoo Dec 18, 2017

Contributor
  1. Make that throw an exception rather than silently returning.
  2. Figure out the case where this happens and make it not push while it's not the current screen.
Contributor

smoogipoo commented Dec 18, 2017

  1. Make that throw an exception rather than silently returning.
  2. Figure out the case where this happens and make it not push while it's not the current screen.

peppy added some commits Dec 21, 2017

@peppy

peppy approved these changes Dec 21, 2017

@peppy peppy added the code quality label Dec 21, 2017

@peppy peppy added this to the December 2017 milestone Dec 21, 2017

@peppy peppy merged commit 43b15bf into ppy:master Dec 21, 2017

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@DevSDK DevSDK deleted the DevSDK:osudev branch Dec 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment