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

Enable mouse input for uilist #41272

Closed
wants to merge 13 commits into from

Conversation

martinrhan
Copy link
Contributor

@martinrhan martinrhan commented Jun 13, 2020

Summary

SUMMARY: Features "Enable mouse input for uilist"

Purpose of change

This is further work based on #40794
As what I said, I am going to make mouse input available for other things in the game.

Describe the solution

Similar to what I have done in previous PR, I made a struct uilist_entry_drawn_info to record the information of drawing of the entries. Then I added another else if for SELECT input in void uilist::query( bool loop, int timeout ).

Describe alternatives you've considered

N/A

Testing

Windows10, VS, both release and debug build tested, all fine.

Additional context

Also optimized some codes done before.

src/ui.cpp Show resolved Hide resolved
@mlangsdorf mlangsdorf added the Info / User Interface Game - player communication, menus, etc. label Jun 14, 2020
Copy link
Contributor

@mlangsdorf mlangsdorf left a comment

Choose a reason for hiding this comment

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

Having an identical include_point() function defined in two places isn't good practice. Can they both call into a common function?

@@ -52,6 +52,14 @@ struct inventory_entry_drawn_info {
int text_x_start;
int text_x_end;
int y;
bool include_point( point p )const {
if( text_x_start <= p.x &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you please just put this all on one line? General CDDA practice is to put multiple conditions on a single line if the total line length is less than 100 characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hah? but I saw many cases of mutiple lined if conditions

src/ui.h Outdated Show resolved Hide resolved
@martinrhan
Copy link
Contributor Author

Having an identical include_point() function defined in two places isn't good practice. Can they both call into a common function?

maybe I can add a parent class for all the drawn_infos? But I cannot touch computer in week days, further commit will be lagged until next weekend

}
return false;
}
struct inventory_entry_drawn_info : entry_drawn_info {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use entry_drawn_info directly? This inheritance does not add anything to the parent class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in future there may be something added

Copy link
Contributor

Choose a reason for hiding this comment

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

You can always create a new class in the future, but now it does nothing and only clutters up the code. And even if something is to be added in the future, I would recommend adding it to the parent class (any maybe guard it with a flag), so other UIs can also use the new functionality when they need to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean there are a lot of variant UI things in game, else than inventory and uilist, there are pickup, inventory_item_menu, crafting, and whatever, each of them are having different kind of drawing, its just this two drawn info structs are currently not different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

besides, I am also considering to add some new things, such as button.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean there are a lot of variant UI things in game, else than inventory and uilist, there are pickup, inventory_item_menu, crafting, and whatever, each of them are having different kind of drawing

And I'm pretty sure it's a bad thing we'd like to get rid of and have a unified system to display menu stuff instead.

@@ -64,6 +64,9 @@ struct uilist_entry_drawn_info {
}
};

struct uilist_entry_drawn_info : entry_drawn_info {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for this inheritance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because the way of drawing for those two kinds of UI thing are not exactly the same, currently the drawn info records are not necessary to to be variant yet, but maybe in future one of them is to be changed.

src/inventory_ui.cpp Outdated Show resolved Hide resolved
Co-authored-by: BevapDin <tho_ki@gmx.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Info / User Interface Game - player communication, menus, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants