Eat menu #33304

akozhevn · 2019-08-17T18:47:34Z

Summary

SUMMARY: Interface "Replacing 'E'ating menu with a better one"

Purpose of change

Current menu is clunky and lacks QOL features

Describe the solution

Base the menu off of advancved inventory, so we get targeting, sorting and filtering for free
Ability to toggle between Food, Drugs - technically can filter with 'c:' but it's too many button presses
CBM opens eating menu when turned on
Add ability to warm up food from the menu
Add ability to cook using current ingredient from the menu

Describe alternatives you've considered

I think creating new menu is easier than adding all these features to current one, because current is generic code, shared between multiple menus.

Additional context

Large + help page

Small + mutation error

Furance menu + sorting

WIP for 'E'ating menu # Explain why this change is being made current one is clunky

fix tests/ errors

Petethegoat · 2019-08-17T18:48:11Z

Can we get some screenshots?

anothersimulacrum · 2019-08-17T19:19:52Z

You need to astyle, and fix the build errors.

src/comestible_inv.cpp

ZhilkinSerg · 2019-08-17T21:14:22Z

I can see that a lot of code is simply a copy of advanced inventory code. Maybe you can use some shared functions to reduce code duplication?

BevapDin · 2019-08-17T21:16:43Z

Base the menu off of advancved inventory, so we get targeting

You mean by copying most of the content of "advanced_inv.cpp" and "advanced_inv.h". Any change to those files will not appear in the new menu and vice versa. This will lead to inconsistencies. It's generally not a good idea to copy that much code with little changes.

settings are saved within a session can actually eat now

bionics have special eating menu when activated can summon crafting menu from eating menu refactoring to make it more modular, future support

fixing CBM menu showing wrong items fixing heat_up not having shortcuts

reorganizing things a little

BevapDin · 2019-09-18T05:55:22Z

src/comestible_inv_listitem.cpp

+    menu_color = it.color_in_inventory();
+
+    // statuses
+    const auto &comest = it.get_comestible();


I assume you meant to get the edible instead of querying it directly here (get_edible_comestible queries it anyway).

I don't know what the difference is between get_edible_comestible( p, it ); and it.get_comestible(); Edible and comestible are synonyms and it's not documented which function I should use when.
This code is copied from different places, so not sure what it actually needs to be. The functions seem different on the inside, so I don't know the consequences if we just replace comest with edible

I don't know what the difference is between

Than look at the source of their implementation.

This code is copied from different places,

Copying code without understanding what it does (or even why it does it) is dangerous.

get_comestible returns an optional value, dereferencing the optional (as you do below via comest->) will cause UB when it does not actually contain a value. Your code does not check this, it just dereferences in the assumption that it will always contain a value. That leads to a crash when encountering non-food items (where comest is empty).

get_edible_comestible (which looks like a straight copy from "game_inventory.cpp", which is bad on its own) checks for the optional value being empty and returns a dummy value in that case.

Btw. this has nothing to do with edible vs comestible.

src/comestible_inv_listitem.cpp

src/comestible_inv.cpp

codemime · 2019-09-22T10:14:43Z

3500+ lines of mostly copy-pasted code for a new non-generic menu (in addition to the existing ones) shoved into a single PR? I'd say it's a no-go.

akozhevn · 2019-09-24T15:44:16Z

3500+ lines of mostly copy-pasted code for a new non-generic menu (in addition to the existing ones) shoved into a single PR? I'd say it's a no-go.

Ok, let's start with the problem - inventory-type menus are bad (no sorting, grouping by location is generally useless, no QOL features)
How do we improve? I believe incremental changes to current implementation will give mediocre results at best. So I created new base classes.
The code is generic and is only copy pasted if you ignore the overall structure. With it you can create more menus pretty easy. Existing implementation of advanced inventory (ai) doesn't support extension, so you would have to add a lot of these if(type1){} else if(type2){} ... for every new ai-looking menu. Would become a mess fast.
@codemime @BevapDin How about I create new PR, where I just refactor ai? So we can make menus based of that.

kevingranade · 2019-09-25T02:04:19Z

I believe incremental changes to current implementation will give mediocre results at best.

That is not how that works, writing all-new code is easier, but it's also massively riskier. You've identified issues you have with the existing code, but I guarantee that the new code has it's own issues, if nothing else missing features compared to the old code.

The only sure way to prevent this is to incrementally adjust the existing code over time so that bugs and feature omissions are easier to spot and correct.

Cleaning up the advanced inventory code is something we would love soneone to tackle.

ThomasLinkin · 2019-09-25T14:34:52Z

True
I think you should change the existing code instead of creating a whole new one.

ZhilkinSerg · 2019-10-03T07:02:57Z

src/comestible_inv.h

@@ -0,0 +1,104 @@
+#pragma once
+#ifndef comestible_inv_H


Suggested change

#ifndef comestible_inv_H

#ifndef COMESTIBLE_INV_H

ZhilkinSerg · 2019-10-03T07:03:04Z

src/comestible_inv.h

@@ -0,0 +1,104 @@
+#pragma once
+#ifndef comestible_inv_H
+#define comestible_inv_H


Suggested change

#define comestible_inv_H

#define COMESTIBLE_INV_H

ZhilkinSerg · 2019-10-03T07:03:57Z

src/crafting.h

@@ -14,6 +15,6 @@ void remove_ammo( item &dis_item, player &p );
 // same as above but for each item in the list
 void remove_ammo( std::list<item> &dis_items, player &p );

-const recipe *select_crafting_recipe( int &batch_size );
+const recipe *select_crafting_recipe( int &batch_size, const std::string &filter = "" );


Suggested change

const recipe *select_crafting_recipe( int &batch_size, const std::string &filter = "" );

const recipe *select_crafting_recipe( int &batch_size, const std::string &filter );

ZhilkinSerg · 2019-10-03T07:04:42Z

src/player.h

@@ -1424,7 +1424,7 @@ class player : public Character
         * Start various types of crafts
         * @param loc the location of the workbench. tripoint_zero indicates crafting from inventory.
         */
-        void craft( const tripoint &loc = tripoint_zero );
+        void craft( const tripoint &loc = tripoint_zero, const std::string &filter = "" );


Suggested change

void craft( const tripoint &loc = tripoint_zero, const std::string &filter = "" );

void craft( const tripoint &loc = tripoint_zero, const std::string &filter );

ZhilkinSerg · 2019-10-03T07:05:39Z

src/comestible_inv_listitem.cpp

+
+    eat_error = p.can_eat( it );
+
+    exipres_in = "";


Suggested change

exipres_in = "";

exipres_in.clear();

ZhilkinSerg · 2019-10-03T07:05:51Z

src/comestible_inv_listitem.cpp

+    eat_error = p.can_eat( it );
+
+    exipres_in = "";
+    shelf_life = "";


Suggested change

shelf_life = "";

shelf_life.clear();

ZhilkinSerg · 2019-10-03T07:06:04Z

src/comestible_inv_listitem.cpp

+{
+    //Function is overriden in children. Code is to supress warnings/errors.
+    if( col == 0 ) {
+        print_str = "";


Suggested change

print_str = "";

print_str.clear();

ZhilkinSerg · 2019-10-03T07:06:13Z

src/comestible_inv_listitem.cpp

+{
+    //Function is overriden in children. Code is to supress warnings/errors.
+    if( col == 0 ) {
+        print_str = "";


Suggested change

print_str = "";

print_str.clear();

ZhilkinSerg · 2019-10-03T07:06:22Z

src/comestible_inv_listitem.cpp

+                }
+                print_str = string_format( "%4d", stacks );
+            } else {
+                print_str = "";


Suggested change

print_str = "";

print_str.clear();

ZhilkinSerg · 2019-10-03T07:07:09Z

src/comestible_inv_listitem.cpp

+        string_format = "%d";
+    } else {
+        print_color = need_highlight ? c_yellow : c_dark_gray;
+        string_format = "";


Suggested change

string_format = "";

string_format.clear();

stale · 2019-11-12T08:15:23Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

kevingranade · 2020-04-24T02:23:10Z

This large scale overhaul isn't going to happen in one shot, the options are incremental change or stick with the status quo.

akozhevn added 2 commits August 17, 2019 13:16

# If this commit is applied, it will ....

910fee4

WIP for 'E'ating menu # Explain why this change is being made current one is clunky

# If this commit is applied, it will ....

ffa2196

fix tests/ errors

ZhilkinSerg reviewed Aug 17, 2019

View reviewed changes

src/comestible_inv.cpp Outdated Show resolved Hide resolved

ZhilkinSerg reviewed Aug 17, 2019

View reviewed changes

src/comestible_inv.cpp Outdated Show resolved Hide resolved

ZhilkinSerg reviewed Aug 17, 2019

View reviewed changes

src/comestible_inv.cpp Outdated Show resolved Hide resolved

ZhilkinSerg reviewed Aug 17, 2019

View reviewed changes

src/comestible_inv.cpp Outdated Show resolved Hide resolved

ZhilkinSerg reviewed Aug 17, 2019

View reviewed changes

src/comestible_inv.cpp Outdated Show resolved Hide resolved

ZhilkinSerg reviewed Aug 17, 2019

View reviewed changes

src/comestible_inv.cpp Outdated Show resolved Hide resolved

akozhevn added 13 commits August 17, 2019 17:22

Merge branch 'master' into eatMenu

7d6b166

clean up the code

1fcc39e

settings are saved within a session can actually eat now

# If this commit is applied, it will ....

7839d41

bionics have special eating menu when activated can summon crafting menu from eating menu refactoring to make it more modular, future support

Merge branch 'master' into eatMenu

00f3ece

astyle

9016492

travis error

e6628ad

travis error2

fbf078a

travis error 3

e63bdad

fixing CBM menu showing wrong items fixing heat_up not having shortcuts

astyle

c749baf

Merge branch 'master' into eatMenu

d71810a

update for translation in bionics

4bb63da

adding shortcuts

3a1d5aa

reorganizing things a little

travis error 4

c2587ff

BevapDin reviewed Sep 18, 2019

View reviewed changes

src/comestible_inv_listitem.cpp Outdated Show resolved Hide resolved

BevapDin reviewed Sep 18, 2019

View reviewed changes

src/comestible_inv.cpp Outdated Show resolved Hide resolved

ZhilkinSerg self-assigned this Sep 19, 2019

fixes from BevapDin

c37364f

weekly-digest bot mentioned this pull request Sep 29, 2019

Weekly Digest (22 September, 2019 - 29 September, 2019) #34319

Closed

ZhilkinSerg reviewed Oct 3, 2019

View reviewed changes

akozhevn mentioned this pull request Oct 6, 2019

[WIP]advanced_inv refactor #34502

Closed

3 tasks

ZhilkinSerg removed their assignment Oct 13, 2019

stale bot added the stale Closed for lack of activity, but still valid. label Nov 12, 2019

ZhilkinSerg added 0.E Feature Freeze and removed stale Closed for lack of activity, but still valid. labels Nov 12, 2019

weekly-digest bot mentioned this pull request Dec 29, 2019

Weekly Digest (22 December, 2019 - 29 December, 2019) #36542

Closed

ifreund removed the 0.E Feature Freeze label Apr 4, 2020

kevingranade closed this Apr 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eat menu #33304

Eat menu #33304

akozhevn commented Aug 17, 2019 •

edited

Loading

Petethegoat commented Aug 17, 2019

anothersimulacrum commented Aug 17, 2019

ZhilkinSerg commented Aug 17, 2019

BevapDin commented Aug 17, 2019

BevapDin Sep 18, 2019

akozhevn Sep 20, 2019

BevapDin Sep 20, 2019

codemime commented Sep 22, 2019

akozhevn commented Sep 24, 2019

kevingranade commented Sep 25, 2019

ThomasLinkin commented Sep 25, 2019

ZhilkinSerg Oct 3, 2019

ZhilkinSerg Oct 3, 2019

ZhilkinSerg Oct 3, 2019

ZhilkinSerg Oct 3, 2019

ZhilkinSerg Oct 3, 2019

ZhilkinSerg Oct 3, 2019

ZhilkinSerg Oct 3, 2019

ZhilkinSerg Oct 3, 2019

ZhilkinSerg Oct 3, 2019

ZhilkinSerg Oct 3, 2019

stale bot commented Nov 12, 2019

kevingranade commented Apr 24, 2020

	const recipe *select_crafting_recipe( int &batch_size, const std::string &filter = "" );
	const recipe *select_crafting_recipe( int &batch_size, const std::string &filter );

	void craft( const tripoint &loc = tripoint_zero, const std::string &filter = "" );
	void craft( const tripoint &loc = tripoint_zero, const std::string &filter );

Eat menu #33304

Eat menu #33304

Conversation

akozhevn commented Aug 17, 2019 • edited Loading

Summary

Purpose of change

Describe the solution

Describe alternatives you've considered

Additional context

Petethegoat commented Aug 17, 2019

anothersimulacrum commented Aug 17, 2019

ZhilkinSerg commented Aug 17, 2019

BevapDin commented Aug 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codemime commented Sep 22, 2019

akozhevn commented Sep 24, 2019

kevingranade commented Sep 25, 2019

ThomasLinkin commented Sep 25, 2019

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

Choose a reason for hiding this comment

stale bot commented Nov 12, 2019

kevingranade commented Apr 24, 2020

akozhevn commented Aug 17, 2019 •

edited

Loading