kosenko / ui

Boost.UI library
266 stars 18 forks source link

events handled incorrectly #25

Closed UltraBlackLinux closed 1 year ago

UltraBlackLinux commented 1 year ago

Hey there, I just discovered an issue with events. Consider the following code:

class Product {
private:
    ui::window parent;
    ui::label name;
    ui::string_box price;
    ui::choice unit;
public:
    Product(ui::window& iparent, const std::string& name) : parent{iparent}, name{ui::label(iparent, name)} {
    price = ui::string_box(parent, "Price");

    price.on_edit([&](){
       price.text(std::regex_replace(price.text().string(), std::regex("[^\\d\.]"), ""));
       priceStatus.text("");
    });
    unit = ui::choice(parent, {"g", "kg"});
    unit.select(0);
    }
    ui::layout::item layout() {
    return (ui::hbox() << name << price << unit).layout().center();
    }
    float getPrice() {
    float outPrice = std::stof(price.text().string());
    if (unit.current_selection_index() == 0) {
       outPrice *= 1000;
    }
    return outPrice;
    }
};

If I now create instances of this class, and type in any text box it turns out that only the last created instance gets this regex applied. I don't think this is an issue on my side, but please tell me, if it is! Thanks!

kosenko commented 1 year ago

I can't repeat your problem. price class variable was created inside class and should not collide with price variables in other instances of this class. Your problem could be in priceStatus variable that is not related to Product class.

#include <boost/ui.hpp>
#include <regex>
namespace ui = boost::ui;

class Product {
private:
    ui::window parent;
    ui::label name;
    ui::string_box price;
    ui::choice unit;
public:
    Product(ui::window& iparent, const std::string& name) : parent{iparent}, name{ui::label(iparent, name)} {
        price = ui::string_box(parent, "Price");

        price.on_edit([&](){
            price.text(std::regex_replace(price.text().string(), std::regex("[^\\d\.]"), ""));
            //priceStatus.text("");
        });
        unit = ui::choice(parent, {"g", "kg"});
        unit.select(0);
    }
    ui::layout::item layout() {
        return (ui::hbox() << name << price << unit).layout().center();
    }
    float getPrice() {
        float outPrice = std::stof(price.text().string());
        if (unit.current_selection_index() == 0) {
            outPrice *= 1000;
        }
        return outPrice;
    }
};

int ui_main()
{
    ui::dialog dlg("Products");
    Product product1(dlg, "product 1");
    Product product2(dlg, "product 2");
    ui::vbox(dlg)
        << product1.layout()
        << product2.layout()
        << ui::button(dlg, "Show prices")
            .on_press([&] {
                std::wostringstream ss;
                ss << product1.getPrice() << " " << product2.getPrice();
                ui::info_dialog(ss.str());
            })
        ;
    dlg.show_modal();
    return 0;
}

int main(int argc, char* argv[])
{
    return ui::entry(&ui_main, argc, argv);
}
UltraBlackLinux commented 1 year ago

bin.zip If you feel comfortable running randomass binaries, then take a look at the one in the zip file. The code is slightly updated and I'm still facing the same issue.

The code for that is here

EDIT: I just found the issue for two boxes always getting the same input - I forgot to change one variable while copying that over. The problem with the top box still not being fixed but the bottom one being is still weird

kosenko commented 1 year ago

Here is your code

#include <algorithm>
#include <bits/stdc++.h>
#include <boost/ui.hpp>
#include <boost/ui/button.hpp>
#include <boost/ui/label.hpp>
#include <boost/ui/layout.hpp>
#include <boost/ui/menu.hpp>
#include <boost/ui/status_bar.hpp>
#include <boost/ui/strings_box.hpp>
#include <boost/ui/window.hpp>
#include <iostream>
#include <math.h>
#include <regex.h>
#include <vector>

namespace ui = boost::ui;
ui::label priceStatus;

class Product {
private:
  ui::window parent;
  ui::label name;
  ui::string_box price;
  ui::string_box weight;
  ui::choice unit;

public:
  Product(ui::window &iparent, const std::string &name)
      : parent{iparent}, name{ui::label(iparent, name)} {
    price = ui::string_box(parent, "Price");
    price.on_edit([&]() {
      price.text(std::regex_replace(price.text().string(),
                    std::regex("[^\\d\.]"), ""));
      priceStatus.text("");
    });
    weight = ui::string_box(parent, "Weight");
    weight.on_edit([&]() {
      weight.text(std::regex_replace(price.text().string(),
                     std::regex("[^\\d\.]"), ""));
    });

    unit = ui::choice(parent, {"g", "kg"});
    unit.select(0);
  }
  ui::layout::item layout() {
    return (ui::hbox() << name << price << ui::label(parent, "E") << weight
                       << unit)
        .layout()
        .center();
  }
  float getPrice() {
    float outPrice = std::stof(price.text().string());
    float outWeight = std::stof(weight.text().string());
    if (unit.current_selection_index() == 0) {
      outWeight /= 1000;
    }
    return outPrice / outWeight;
  }
};

std::vector<Product> products{};

void comparePrices() {
  int lowestPrice = 999999;
  int bestIndex = 0;
  int index = 0;
  for (Product p : products) {
    if (p.getPrice() < lowestPrice) {
      lowestPrice = p.getPrice();
      bestIndex = index;
    }
    ++index;
  }
  priceStatus.text("Product " + std::to_string(bestIndex) +
                   " is the cheapest.");
}

// class Window : public ui::frame {
// public:
//     Window() : ui::frame() {
//     }
// };

void window() {
  ui::frame parent = ui::frame();
  parent.create("Price Comparison");
  // parent.status_bar().text("");
  priceStatus = ui::label(parent, "");
  ui::vbox layout(parent);
  for (int i = 0; i < 2; ++i) {
    products.push_back(Product(parent, "Product " + std::to_string(i)));
  }

  for (auto it = products.begin(); it != products.end(); ++it) {
    layout << it->layout();
  }
  layout << priceStatus.layout().hcenter()
         << ui::button(parent, "Calculate")
                .on_press(comparePrices)
                .layout()
                .hcenter();

  parent.show_modal();
}

int run_window() {
  window();
  return 0;
}

int main(int argc, char *argv[]) { return ui::entry(&run_window, argc, argv); }

You are using lambda functions that rely on temporary Product instance (Product::price class variable) that destroys after insert into std::vector<>. You need to disable copy constructor and copy assignment operator or use boost::noncopyable class for this. It is possible to use std::vector<std::unique_ptr<>> to store products. There are hotfixes like moving lambda functions to Product::layout or use Product::price copy, but do it is a bad idea.

UltraBlackLinux commented 1 year ago

You are using lambda functions that rely on temporary Product instance (Product::price class variable) that destroys after insert into std::vector<>. You need to disable copy constructor and copy assignment operator or use boost::noncopyable class for this. It is possible to use std::vector<std::unique_ptr<>> to store products. There are hotfixes like moving lambda functions to Product::layout or use Product::price copy, but do it is a bad idea.

Oh that did it! I feel like I am still somewhat too inexperienced in C++ to fully understand what's going on, but what I now understand, is that the widgets themselves don't apparently store the event functions like I thought they would so it makes total sense that the first created ones don't have this effect applied, but I still don't understand why it did work for the last two. I also don't understand what role unique pointers play here, but whatever. Thanks ;)

kosenko commented 1 year ago

It works for last two widgets because lambda function reads dirty data from deleted Product instance, that was last initialized. std::unique_ptr<> should help to use raw pointers, but you can use directly std::vector<Product*> array.