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.

Sign up for GitHub

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

Jump to bottom

add CBA_fnc_progressBar #912

Merged
merged 19 commits into from
Apr 29, 2018
Merged

add CBA_fnc_progressBar #912

merged 19 commits into from
Apr 29, 2018

Conversation

commy2
Copy link
Contributor

@commy2 commy2 commented Apr 21, 2018 •
edited
Loading

When merged this pull request will:

  • title

@commy2 commy2 added the Feature label Apr 21, 2018
PabstMirror
PabstMirror reviewed Apr 21, 2018
View reviewed changes
addons/ui/fnc_progressBar.sqf Outdated
Opens a progress bar. Closes the currently active progress bar.

Parameters:
_condition - Execute every frame. If reports false, close the progress bar <CODE>
Copy link
Contributor

Choose a reason for hiding this comment

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

_title

Cuel
Cuel reviewed Apr 21, 2018
View reviewed changes
addons/ui/fnc_progressBar.sqf Outdated
@@ -0,0 +1,168 @@
/* ----------------------------------------------------------------------------
Function: CBA_fnc_display
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong name

Cuel
Cuel reviewed Apr 22, 2018
View reviewed changes
addons/ui/fnc_progressBar.sqf Outdated

Examples:
(begin example)
["progress bar", {true}, 5, {hint "done"}, {hint "aborted"}] call CBA_fnc_display;
Copy link
Contributor

@Cuel Cuel Apr 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

wrong function name

@commy2
Copy link
Contributor Author

commy2 commented Apr 22, 2018

ref: https://github.com/acemod/ACE3/blob/master/addons/common/functions/fnc_progressBar.sqf

commy2
commy2 commented Apr 22, 2018
View reviewed changes
addons/ui/fnc_progressBar.sqf Outdated
} else {
QGVAR(ProgressBar) cutRsc [QGVAR(ProgressBar), "PLAIN"];
_display = uiNamespace getVariable QGVAR(ProgressBar);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

todo, MouseMoving, MouseHolding don't work for cutRsc.

@commy2 commy2 added the WIP label Apr 22, 2018
kymckay
kymckay reviewed Apr 22, 2018
View reviewed changes
addons/ui/fnc_progressBar.sqf Outdated
if (!hasInterface) exitWith {};

if (canSuspend) exitWith {
isNil (uiNamespace getVariable _fnc_scriptName);
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this bit all about?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Run the function unscheduled every time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as https://github.com/CBATeam/CBA_A3/blob/master/addons/settings/fnc_init.sqf#L44-L47

Copy link
Contributor

@dedmen dedmen Apr 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe prefer fnc_directCall for clarity?
I'd usually push the performance argument.. But this is scheduled soooooo....

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I just hadn't see the uiNamespace stuff before, was curious as to how that worked

PabstMirror
PabstMirror reviewed Apr 23, 2018
View reviewed changes
addons/ui/fnc_progressBar.sqf Outdated
[_onFailure, [_arguments, false, _elapsedTime, _totalTime]] call CBA_fnc_execNextFrame;
};

if (_elapsedTime > _totalTime) exitWith {
Copy link
Contributor

Choose a reason for hiding this comment

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

_elapsedTime = (CBA_missionTime - _startTime) min _totalTime;
so elapsed can never be greater than total

@commy2
Copy link
Contributor Author

commy2 commented Apr 23, 2018

Any suggestion on how to order those parameters? -__-

@commy2 commy2 removed the WIP label Apr 23, 2018
@dedmen
Copy link
Contributor

dedmen commented Apr 23, 2018

onFailure could be optional. Most people probably don't need that.
And i would swap condition and totalTime.
So that you have all code variables together.

@commy2
Copy link
Contributor Author

commy2 commented Apr 23, 2018 •
edited
Loading

In truth, all params are optional, but I will edit the header.

@commy2 commy2 added the WIP label Apr 24, 2018
@commy2 commy2 removed the WIP label Apr 24, 2018
@dedmen
Copy link
Contributor

dedmen commented Apr 25, 2018

Why the change from a small onDraw EH to a big, always recompiling EachFrame EH?
Yeah it doesn't matter if your fps are one lower in a progress bar. You can't do anything anyway. But why?

@commy2
Copy link
Contributor Author

commy2 commented Apr 25, 2018

Latest is using onDraw.
I changed for a short while, because

  • EF cerries over from savegames (fixed by using Loaded event checking a global var that also contains the args)
  • killing the display from inside onDraw ctd's (fixed by execNextFrame, race conditions?)

@dedmen
Copy link
Contributor

dedmen commented Apr 25, 2018 •
edited
Loading

race conditions?

The onDraw EH runs in the middle of the Drawing function of the map control, not at the end. So if you delete the map who'se function is currently running and the function then tries to continue drawing the map and access it's own variables.. boom.

@commy2
Copy link
Contributor Author

commy2 commented Apr 25, 2018

Not what I meant. I want to make sure this stuff doesn't behave weird if you e.g. create a new progress bar at the same time the previous one closes.

@commy2
Copy link
Contributor Author

commy2 commented Apr 26, 2018

Ready now hopefully.

kymckay
kymckay reviewed Apr 26, 2018
View reviewed changes
addons/ui/fnc_progressBar.sqf Outdated
@@ -89,4 +80,70 @@ if (!isNil QGVAR(ProgressBarParams)) then {

GVAR(ProgressBarParams) = [_arguments, _condition, _onSuccess, _onFailure, CBA_missionTime, _totalTime, _blockMouse, _blockKeys, _allowClose];
Copy link
Contributor

Choose a reason for hiding this comment

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

Not that it really matters, but you could set these on the display itself and that way it get's cleaned up automatically

Copy link
Contributor Author

@commy2 commy2 Apr 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The display dies when saving and loading a savegame.
If I use a global instead, which is serialized properly, I can create a new loading bar with the same parameters once the game is loaded:
https://github.com/CBATeam/CBA_A3/pull/912/files#diff-8ff25e6d6abfb5db687c39f11a07e5b4R7

Only thing that doesn't work is the block input display, but I just assume that this only ever happens to mouse enabled progress bars, since only then you can use Esc to open the menu to save.

PabstMirror
PabstMirror reviewed Apr 27, 2018
View reviewed changes
addons/ui/RscTitles.hpp Outdated
};

class Script: RscMapControl {
idc = IDC_PROGRESSBAR_SCRIPT
Copy link
Contributor

Choose a reason for hiding this comment

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

File x\cba\addons\ui\RscTitles.hpp, line 51: '/RscTitles/cba_ui_ProgressBar/Script.idc': Missing ';' at the end of line

@Killswitch00
Copy link
Contributor

Guys, get me a "+1" on this one if you feel it's ready.

@Killswitch00 Killswitch00 merged commit b982e79 into master Apr 29, 2018
@commy2 commy2 deleted the progressBar branch April 29, 2018 09:04
@dedmen dedmen mentioned this pull request Jun 26, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dedmen dedmen dedmen left review comments

@kymckay kymckay kymckay left review comments

@Cuel Cuel Cuel left review comments

@PabstMirror PabstMirror PabstMirror left review comments

Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@commy2 @dedmen @Killswitch00 @kymckay @Cuel @PabstMirror