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

Fix retry count not displaying on pause overlay #1709

Merged
merged 3 commits into from Dec 21, 2017

Conversation

2 participants
@DevSDK
Contributor

DevSDK commented Dec 18, 2017

I've find "You've retried xx time(s)" message that something weird.
That is not displayed pause overlay and only see count on FailOverlay
I change that code that PauseContainer.Retries property can be set call-back function.

@DevSDK DevSDK changed the title from Fix Retrycounter only display FailOverlay to Fix Not update retry counter on PauseOverlay Dec 18, 2017

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Dec 19, 2017

Member

A more correct fix for this would be to resolve the implementation in GaameplayMenuOverlay:

image

Member

peppy commented Dec 19, 2017

A more correct fix for this would be to resolve the implementation in GaameplayMenuOverlay:

image

@DevSDK

This comment has been minimized.

Show comment
Hide comment
@DevSDK

DevSDK Dec 20, 2017

Contributor

Why? I want more detail information..
I don't know what is wrong with my PR.
Could you advice to be about detail of your suggestion.

Contributor

DevSDK commented Dec 20, 2017

Why? I want more detail information..
I don't know what is wrong with my PR.
Could you advice to be about detail of your suggestion.

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Dec 20, 2017

Member

The issue is that setting Retries too early sets nothing, due to that null check in my screenshot. This is incorrect – properties should set their values regardless of the instance's state.

A possible solution would be storing retries to a local private int retries in the case the container is null, then perform initialisations in a separate method that is called both in that property (in the non-null case) and also in load() or LoadComplete()

Member

peppy commented Dec 20, 2017

The issue is that setting Retries too early sets nothing, due to that null check in my screenshot. This is incorrect – properties should set their values regardless of the instance's state.

A possible solution would be storing retries to a local private int retries in the case the container is null, then perform initialisations in a separate method that is called both in that property (in the non-null case) and also in load() or LoadComplete()

@DevSDK

This comment has been minimized.

Show comment
Hide comment
@DevSDK

DevSDK Dec 20, 2017

Contributor

Well I've try to change my code.
If retryCounterContaineris null value or not, always storing private int retries .
So, Retries property set value by retries member value and that point retryCounterContainer is not set null.
I hope my code change is right.
Thx your opinion.
:)

Contributor

DevSDK commented Dec 20, 2017

Well I've try to change my code.
If retryCounterContaineris null value or not, always storing private int retries .
So, Retries property set value by retries member value and that point retryCounterContainer is not set null.
I hope my code change is right.
Thx your opinion.
:)

DevSDK and others added some commits Dec 18, 2017

Fix Not update retry counter on PauseOverlay
I've find "You've retried xx time(s)" message that something weird.
That is not displayed pause overlay and only see count on FailOverlay
I change code that PauseContainer.Retries property can be set call-back function.

Signed-off-by: Seokho Song <0xdevssh@gmail.com>

@DevSDK DevSDK closed this Dec 21, 2017

@DevSDK

This comment has been minimized.

Show comment
Hide comment
@DevSDK

DevSDK Dec 21, 2017

Contributor

Oh well, I've change my code :)
Thx for advice!

Contributor

DevSDK commented Dec 21, 2017

Oh well, I've change my code :)
Thx for advice!

@DevSDK DevSDK reopened this Dec 21, 2017

@peppy peppy changed the title from Fix Not update retry counter on PauseOverlay to Fix retry count not displaying on pause overlay Dec 21, 2017

@peppy peppy dismissed their stale review Dec 21, 2017

.

@peppy

peppy approved these changes Dec 21, 2017

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

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@DevSDK DevSDK deleted the DevSDK:dev-overlay-counter branch Dec 21, 2017

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