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

Creating controls is unsound #54

Open
NoraCodes opened this issue Dec 18, 2018 · 9 comments
Open

Creating controls is unsound #54

NoraCodes opened this issue Dec 18, 2018 · 9 comments
Labels
c-bug Bug - some feature is not working as expected p-high High Priority
Milestone

Comments

@NoraCodes
Copy link
Collaborator

Creating controls is unsound at the moment. libui enforces a tree structure. We should enforce it in the API.

I would like to do this by creating a trait which permits adding a control, and implementing it on all containers (like windows and grids and boxes).

The question is, should we have individual methods for each control:

let btn: &mut Button = window.add_button("Button text!");

or should we have an enum with all the controls?

let btn: &mut Button = window.add(Controls::Button("Button text!"));
@NoraCodes NoraCodes added c-bug Bug - some feature is not working as expected p-high High Priority labels Dec 18, 2018
@NoraCodes
Copy link
Collaborator Author

It would perhaps be useful to maintain a hashmap within each of these container controls with each component, which the user could name.

This is kind of problematic because we have to decide:

  1. Does the user have to name every control?
  2. What happens when the user gives two controls the same name?

@barzamin
Copy link
Member

by "creating controls is unsound", what exactly do you mean? can you give an example?

@NoraCodes
Copy link
Collaborator Author

NoraCodes commented Dec 23, 2018

When you make a control, if you don't add as a node in a tree whose root is ultimately a Window or other appropriate control prior to UI handle deinit, the process will abort without unwinding, which is (afaik) not sound.

@barzamin
Copy link
Member

barzamin commented Dec 24, 2018 via email

@jonas-schievink
Copy link

Aborting in itself is safe and sound if done deliberately (eg. by a failed assertion) but not if it happens as a result of undefined behaviour (on the Rust or C side). Of course aborting is still suboptimal in general and is better avoided.

@NoraCodes
Copy link
Collaborator Author

I think the solution is to implement a trait for containers (HorizontalBox, VerticalBox, Window, Grid, etc) which provides functions to create new controls as children, thus never allowing controls to exist outside of the tree.

This seems like a nicer API anyway, to me.

@academician
Copy link

Alternatively, controls could be created "lazily", such that they are not actually created until attached to a node.

@NoraCodes
Copy link
Collaborator Author

If we do make an API whereby each container has methods to create its children, whether we do so lazily or eagerly, we could also place a &'ctx UI within each control, which would obviate the need for constantly passing around &'ctx UI. It would only be needed on associated functions without a receiver.

@NoraCodes
Copy link
Collaborator Author

This discussion is potentially relevant to #92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-bug Bug - some feature is not working as expected p-high High Priority
Projects
None yet
Development

No branches or pull requests

4 participants