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

Common - Add support for non-blocking progress bar #9493

Merged
merged 8 commits into from Nov 11, 2023

Conversation

LinkIsGrim
Copy link
Contributor

@LinkIsGrim LinkIsGrim commented Oct 15, 2023

When merged this pull request will:

  • Title.
  • Make Cargo's airdropping use it.

This does not replace the current progress bar functionality (dialog).

Pressing Escape/opening a dialog/opening the interaction menu still closes the progress bar. Is there anything else that needs to be handled by this?

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Development Guidelines are read, understood and applied.
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

@LinkIsGrim LinkIsGrim added the kind/feature Release Notes: **ADDED:** label Oct 15, 2023
@amsteadrayle
Copy link
Contributor

Is there a reason that the progress bar intercepts mouse movement? Not being able to freelook while in progress has been a little pet peeve of mine for a long time.

@LinkIsGrim
Copy link
Contributor Author

The progress bar is a UI dialog. Dialogs and displays intercept mouse movement. That's about it.

@veteran29 veteran29 self-requested a review October 18, 2023 20:51
@LinkIsGrim
Copy link
Contributor Author

LinkIsGrim commented Oct 19, 2023

I can't find a way to avoid the fade-in, "PLAIN NOFADE" has no effect on cutRsc.

Didn't edit the function header. I'm lazy.

Interactions with animations will still block player movement even with the new param. Freelook is unaffected.

Interactions without animations will allow player movement/vehicle control freely. Anyone using this will need to embed that in condition.

Again, this has to be used explicitly. Default behavior is the dialog. This PR only modifies airdropping to use this.

@johnb432
Copy link
Contributor

johnb432 commented Nov 10, 2023

How well does it handle death?

Didn't edit the function header. I'm lazy.

You should add it.


I really don't like the fading in, but I don't know how to fix it. Considering it's used in 1 instance only, I think we can, at least for now, ignore that.

Copy link
Member

@veteran29 veteran29 left a comment

Choose a reason for hiding this comment

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

LGTM

@LinkIsGrim
Copy link
Contributor Author

Gonna edit the header. Hold on merge till then.

@LinkIsGrim
Copy link
Contributor Author

How well does it handle death?

Should be same as the regular progress bar handles it, but haven't tested for that specifically.

@LinkIsGrim
Copy link
Contributor Author

Note for changelog, add note on airdrop change.

@LinkIsGrim LinkIsGrim added this to the 3.16.2 milestone Nov 11, 2023
Co-authored-by: johnb432 <58661205+johnb432@users.noreply.github.com>
@LinkIsGrim
Copy link
Contributor Author

Removing the key handler twice doesn't cause any issues, thankfully.

Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

Noticed during testing that pressing the "Airdrop" button would sometimes fail silently. I can't get a proper repro though,
I assume this will be merged before #9617, which will give us more time to determine what the issue is.

@LinkIsGrim
Copy link
Contributor Author

Likely failed due to flight conditions, notification should pop up though.

@LinkIsGrim LinkIsGrim merged commit 3fcef89 into acemod:master Nov 11, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Release Notes: **ADDED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants