Expose progress bar widget to scripting #22069

Gymnasiast · 2024-05-20T14:31:44Z

Todo:

Testing
Updating openrct2.d.ts
Move bitshifting/masking to functions/methods
Does colour need to be exposed manually, or is that part of the default specs already (like size and position is)?

ltsSmitty · 2024-05-20T14:43:45Z

I also started working on this, so I appreciate it. I'm not sure how to piggyback on this draft but I've got some additional work done.

I'm not sure it makes sense to use text for the value since it's numerical; I'm thinking to use progress instead.
There's also work to be done in CustomWindow.cpp, which handles binding the onChange and the other things to the custom widgetDesc.

Would you like me to contribute to this or are you gonna keep working?

Gymnasiast · 2024-05-20T14:45:46Z

I also started working on this, so I appreciate it. I'm not sure how to piggyback on this draft but I've got some additional work done.
* I'm not sure it makes sense to use `text` for the value since it's numerical; I'm thinking to use `progress` instead.

* There's also work to be done in CustomWindow.cpp, which handles binding the onChange and the other things to the custom widgetDesc.
Would you like me to contribute to this or are you gonna keep working?

I hardly know what I’m doing, so I would like to pick the former. I’ll see if I can give you rights to write to this branch.

Edit: I have now given you access. Feel free to modify the code I already added as you see fit.

Sadret

Looks like a good start, but there is still a lot of implementation to do, see e.g.
https://github.com/OpenRCT2/OpenRCT2/blob/develop/src/openrct2-ui/scripting/ScWidget.hpp#L69
https://github.com/OpenRCT2/OpenRCT2/blob/develop/src/openrct2-ui/scripting/CustomWindow.cpp#L93
https://github.com/OpenRCT2/OpenRCT2/blob/develop/src/openrct2-ui/scripting/CustomWindow.cpp#L942
and maybe other places.

Edit: Didn't see your discussion until after my review.

Sadret · 2024-05-20T15:26:46Z

distribution/openrct2.d.ts

@@ -4277,11 +4277,11 @@ declare global {
     */
    type WidgetType =
        "button" | "checkbox" | "colourpicker" | "custom" | "dropdown" | "groupbox" |
-        "label" | "listview" | "spinner" | "textbox" | "viewport";
+        "label" | "listview" | "spinner" | "textbox" | "viewport" | "progress_bar";


This should be alphabetically ordered.
To follow the naming convention, it should be called progressbar.

Sadret · 2024-05-20T15:26:55Z

distribution/openrct2.d.ts


    type Widget =
        ButtonWidget | CheckboxWidget | ColourPickerWidget | CustomWidget | DropdownWidget | GroupBoxWidget |
-        LabelWidget | ListViewWidget | SpinnerWidget | TextBoxWidget | ViewportWidget;
+        LabelWidget | ListViewWidget | SpinnerWidget | TextBoxWidget | ViewportWidget | ProgressBarWidget;


Sadret · 2024-05-20T15:29:11Z

distribution/openrct2.d.ts

@@ -4363,6 +4363,21 @@ declare global {
        textAlign: TextAlignment;
    }

+    interface ProgressBarWidget extends WidgetBase {


Alphabetical order. Note that SortOrder is just a helper for list view.

Sadret · 2024-05-20T15:32:01Z

distribution/openrct2.d.ts

+        /**
+         * The percentage of the bar to fill, ranging from 0-100 (inclusive).
+         */
+        percentage: number;


For the API I would prefer a fraction between 0 and 1, but I see that it is a percentage internally, so I'm fine with it.

I agree standardization is key here. The only existing use of percent in the api is for ride downtime, which is represented from 0-100. I'm in favor of keeping that structure for now.

distribution/openrct2.d.ts

Sadret · 2024-05-20T15:35:22Z

distribution/openrct2.d.ts

+         * Used to specify a range of values between which the bar will start blinking.
+         * Set both of these to the same value to disable blinking entirely.
+         * The range is inclusive.
+         */


This mechanism is weird. Doesn't setting to -1 and 101 also mean there is no blinking? Anyways, I would suggest null values to hide the implementation details.

You cannot set it to -1, it’s an unsigned integer.

Makes sense. What about 0? Then an empty bar would blink, but you cannot see that :P
But again, I would prefer null values in both getter and setter.

Sadret · 2024-05-20T15:44:35Z

Todo: Create ProgressBarDesc and add it to WidgetDesc in the docs.

ltsSmitty · 2024-05-21T01:31:18Z

src/openrct2-ui/scripting/CustomWindow.cpp

+                widget.type = WindowWidgetType::ProgressBar;
+                if (desc.IsDisabled)
+                {
+                    widget.colour = 1;


I wasn't sure what to do for disabled state, so I went ahead and set it to white and not blinking. Some screenshots of my attempts are in discord.

after all the feedback, decided to just remove any isDisabled functionality and leave it up to the implementer to do, whether they want to recolor, hide, etc.

ltsSmitty · 2024-05-21T01:37:56Z

Here's a simple plugin to see the capabilities.

(function(){'use strict';
var startup = function () {
    if (typeof ui !== "undefined") {
        var menuItemName = "Progress Bar";
        ui.registerMenuItem(menuItemName, open);
    }
};
var open = function () {
    ui.openWindow(windowDesc);
};
var windowDesc = {
    title: "Progress Bar",
    width: 200,
    height: 120,
    colours: [1, 1],
    widgets: [
        {
            type: "progressbar",
            width: 150,
            height: 20,
            x: 10,
            y: 25,
            colour: 30,
            percentage: 90,
            lowerBlinkBound: 80,
            upperBlinkBound: 20
        },
        {
            type: "progressbar",
            width: 100,
            height: 10,
            x: 10,
            y: 70,
            colour: 7,
            percentage: 80,
            lowerBlinkBound: 67,
            upperBlinkBound: 100
        },
        {
            type: "progressbar",
            width: 100,
            height: 10,
            x: 10,
            y: 90,
            colour: 50,
            percentage: 10,
            isDisabled: true
        }
    ],
    classification: "test"
};
registerPlugin({
    name: "Progress Bar Test",
    version: "1.0",
    authors: ["ltsSmitty"],
    type: "remote",
    licence: "MIT",
    targetApiVersion: 70,
    main: startup,
});})();

mrmagic2020 · 2024-05-21T06:12:32Z

Tested the implementation in my plugin and it seems to work fine. Do note that ProgressBarDesc is missing in type WidgetDesc in the API reference. See: https://github.com/OpenRCT2/OpenRCT2/blob/bbb5b92610dbffef1fce680f6948830c08ef9d91/distribution/openrct2.d.ts#L4448C18

Sadret

Looks good, but I would still strongly prefer null values for the blink bounds.

Sadret · 2024-05-21T13:16:40Z

distribution/openrct2.d.ts


    type Widget =
        ButtonWidget | CheckboxWidget | ColourPickerWidget | CustomWidget | DropdownWidget | GroupBoxWidget |
-        LabelWidget | ListViewWidget | SpinnerWidget | TextBoxWidget | ViewportWidget;
+        LabelWidget | ListViewWidget |ProgressBarWidget | SpinnerWidget | TextBoxWidget | ViewportWidget;


space between | and ProgressBarWidget

Sadret · 2024-05-21T13:45:55Z

src/openrct2-ui/scripting/CustomWindow.cpp

-        uint8_t LowerBlinkBound{}; // progress bar blink when below
-        uint8_t UpperBlinkBound{}; // progress bar blink when above
+        uint8_t LowerBlinkBound{}; // progress bar will blink when above the lower bound
+        uint8_t UpperBlinkBound{}; // and below upper bound


Wait, what? I thought it was below the lower bound or above the upper bound.

But this way it also makes sense. It's just a bit weird that you cannot make it blink at only one value though: Both bounds are inclusive, but if both are the same, blinking is disabled.

I agree that it's a bit unintuitive. I'll make a note in the .d.ts that the bar will blink when the percentage is between the upper and lower bound values, which I think will clear it up a bit. And I think this confirms keeping a strictly numerical value instead of using null.

ltsSmitty · 2024-05-21T14:38:06Z

distribution/openrct2.d.ts

+        /**
+         * Used to specify a range of values between which the bar will start blinking.
+         * The bar will blink when lowerBlinkBound <= percentage <= upperBlinkBound.
+         * Set both upper and lower bounds to the same value to disable blinking.
+         */
+        lowerBlinkBound: number;


@Sadret how about this for the description?

@ltsSmitty Wouldn’t it be better to make it blink below the lower bound or above the upper bound? I imagine people would normally want the bar to blink either when the value is too low or too high. And if the higher bound is set lower than the lower bound, the bar can blink if the value is in between.

@ltsSmitty This description seems fine.

@mrmagic2020 I agree that it would make more sense; I can imagine more use cases where you want it to blink when the value is very low or very high than cases where you want it to blink in the middle but not at the extreme cases. The problem is that the progress bar widget does not support it, so we would have to rewrite that one, too. That's possible and actually probably not too much work.

Expose progress bar widget to scripting

35197ba

Update openrct2.d.ts for ProgressBarWidget

6390a76

Gymnasiast added the changelog This issue/PR deserves a changelog entry. label May 20, 2024

Sadret suggested changes May 20, 2024

View reviewed changes

ltsSmitty added 2 commits May 20, 2024 21:23

add progressbar rendering

4077ae7

fix comment formatting for linter

3c77b3c

ltsSmitty reviewed May 21, 2024

View reviewed changes

ltsSmitty added 2 commits May 21, 2024 09:12

remove isDisabled implementation for progress bar.

60063a5

alphabetize widget and widgetDesc types

ab4ce1c

Sadret suggested changes May 21, 2024

View reviewed changes

ltsSmitty added 2 commits May 21, 2024 09:23

clarify comment for how progress bar blinks

cdb9718

update contributors

cfac0d8

Sadret reviewed May 21, 2024

View reviewed changes

ltsSmitty added 3 commits May 21, 2024 09:51

clamp percent between 0-100

d2f8f2c

fix missing space

126652d

clarify blinking behaviour in comment

58d944f

ltsSmitty reviewed May 21, 2024

View reviewed changes

Gymnasiast mentioned this pull request Jul 7, 2024

[Plugin API] Feat: Expose ride satisfaction to plugin api #22172

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose progress bar widget to scripting #22069

Expose progress bar widget to scripting #22069

Gymnasiast commented May 20, 2024 •

edited

Loading

ltsSmitty commented May 20, 2024

Gymnasiast commented May 20, 2024 •

edited

Loading

Sadret left a comment •

edited

Loading

Sadret May 20, 2024

Sadret May 20, 2024

Sadret May 20, 2024

Sadret May 20, 2024

ltsSmitty May 20, 2024

Sadret May 20, 2024

Gymnasiast May 20, 2024

Sadret May 20, 2024

Sadret commented May 20, 2024

ltsSmitty May 21, 2024

ltsSmitty May 21, 2024

ltsSmitty commented May 21, 2024

mrmagic2020 commented May 21, 2024

Sadret left a comment

Sadret May 21, 2024

Sadret May 21, 2024 •

edited

Loading

ltsSmitty May 21, 2024

ltsSmitty May 21, 2024

mrmagic2020 May 21, 2024

Sadret May 31, 2024

Expose progress bar widget to scripting #22069

Are you sure you want to change the base?

Expose progress bar widget to scripting #22069

Conversation

Gymnasiast commented May 20, 2024 • edited Loading

ltsSmitty commented May 20, 2024

Gymnasiast commented May 20, 2024 • edited Loading

Sadret left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sadret commented May 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltsSmitty commented May 21, 2024

mrmagic2020 commented May 21, 2024

Sadret left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sadret May 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gymnasiast commented May 20, 2024 •

edited

Loading

Gymnasiast commented May 20, 2024 •

edited

Loading

Sadret left a comment •

edited

Loading

Sadret May 21, 2024 •

edited

Loading