[RDY] AIM Updates #12566

DavidKeaton · Jun 7, 2015

When using the AIM_ALL directional menu to place an item, it now defaults to placing in a vehicle storage.
When selecting said tile, it is now the tile displayed for panes[dest] when finished.
Fixes #12542
Fixed loading settings for panes.
Loading AIM now respects whether the pane was in or out of vehicle now.
Now, you can move either one item of a stack at a time, X amount at a time, or all at a time. You can even do this from all areas!
Stack movement now respects charges!
The popup for MOVE_VARIABLE_ITEM now automatically enters no number. I have gotten differing requests on the number to start with, so decided to remove the number altogether! Take that, suggestions!
Removed is_any_of() in enums.h, and used std::any_of() instead.
You can now use AIM_ALL as a source pane and use MOVE_ALL_ITEMS!!! So now, just as @Coolthulhu had suggested ever so long ago, you can move everything into a neat little pile! :-D
Fixed MOVE_VARIABLE_ITEMS with worn items.
Rewrote item movement routine.
Charge based item movement should work as expected now.
add_item() and remove_item() now return the amount of items unable to process.
query_charges() now limits the amount of clothing/armor that you can wear. Two of any given type, and the appropriate constant was added to game_constants.h.
Changed query_charges() message to display how many you have when moving variable items, and added a message if the destination is full already when moving variable items.
A fun little story! :-)

hth!
-Davek

kevingranade · Jun 7, 2015

kevingranade reviewed Jun 7, 2015

View changes

kevingranade · Jun 7, 2015

Sign in to view

-    if(!(lip && rip)) {
-        // draw the player in the center of the map
-        g->u.draw(minimap, g->u.pos(), (lip || rip));
+    /* I now present to you, a story of killer moves and even chiller grooves */


I'm going to have to be a curmudgeon here and say I can't figure out what this is doing because of the variable names.

fair enough :-) I can change it if you'd like.

molkemon · Jun 7, 2015

That story is funky as hell, just one more thing, since I don't want to open another issue for it (I already did once), can you make the m command default to 0/empty instead to the full stack size? (for stuff like batteries, solder etc). If someones uses the m key to begin with, it is clear he does not want to move the entire stack, lest he would just press enter, and having to delete the already filled in max stack number is a bit tedious.

Also, I don't know if this is intended behaviour, but for items that stack with amount (like say copper tubings, scrap metal, most items really), the m key just behaves like the enter key - if I use it on a pane with say vehicle storage, both enter and m will pick up one item at a time, in fact, it is impossible to pick up all of them at once, so you have to hold down enter for quite a while to pick a large amount up, while vice versa, using it in inventory pane, Enter will move ALL the amount of the item to the storage at once, but the m key will pull up the move how many dialogue (which again should default to empty/0) It just seems a bit inconsistant.

BevapDin · Jun 7, 2015

BevapDin reviewed Jun 7, 2015

View changes

BevapDin · Jun 7, 2015

Sign in to view

@@ -29,6 +29,14 @@
 #include <cstdlib>
 #include <cstring>

+static itype_id null_item = "NULL";


For consistency sake, the id of the famous null items is "null" (lowercase).

BevapDin · Jun 7, 2015

BevapDin reviewed Jun 7, 2015

View changes

BevapDin · Jun 7, 2015

Sign in to view

@@ -631,7 +648,12 @@ void advanced_inv_area::init()
    static const std::array<ter_id, 6> ter_water = {
        {t_water_dp, t_water_pool, t_swater_dp, t_water_sh, t_swater_sh, t_sewage}
    };
-    if(is_any_of(g->m.ter(pos), ter_water)) {
+    const auto &map = g->m;
+    auto ter_check = [&map, this]


if you only need pos inside the lambda, you should only capture pos and not this.

g->m is global, no need to capture it, it can be used directly in the lambda.

I am unable to solely grab pos in this lambda, by ref or value. Keeps referring to not having this captured.

Ah, yes, in that case you don't need to access it via this->, just capture this and write ter(pos). The code inside the lambda behaves like it's defined in a member function with implicit usage of this.

BevapDin · Jun 7, 2015

BevapDin reviewed Jun 7, 2015

View changes

BevapDin · Jun 7, 2015

Sign in to view

+            aim_location area, bool from_vehicle);
+    /**
+     * Create a normal item entry.
+     * @param items The list of item pointers, stored in @ref it.
     * @param index The index, stored in @ref idx.
     * @param count The stack size, stored in @ref stacks.


Funny thing, this diff looks quite different when excluding whitespace changes, however:

This function doesn't have the count parameter anymore, please remove the documentation of it.

BevapDin · Jun 7, 2015

BevapDin reviewed Jun 7, 2015

View changes

BevapDin · Jun 7, 2015

src/advanced_inv.cpp

@@ -684,7 +706,7 @@ advanced_inv_listitem::advanced_inv_listitem( item *an_item, int index, int coun
        aim_location _area, bool from_veh )
    : idx( index )
    , area( _area )
-    , it( an_item )
+    , id(an_item->type->id)


(Not tested, only theory) This looks like the items member is not initialized at all, callers seem to rely on the list containing at least one entry. Does that work?

I've had no issue with it, it's pushed back in the constructor.

Ah, yes, I see now.

DavidKeaton · Jun 7, 2015

@molkemon glad you liked the funk, son. ;-)

I've had different people ask for different amounts for the input amount, so what I am going to do is just make the input blank, to enter your own amount.

As far as the key behaviour goes, I have actually split the enter and 'm' key into different functions now. They are as follows:

enter: move single item
m: move item by amount
M: move entire stack

BevapDin · Jun 9, 2015

BevapDin reviewed Jun 9, 2015

View changes

BevapDin · Jun 9, 2015

Sign in to view

+        if( destarea == AIM_INVENTORY ) {
+            auto it = g->u.i_add( new_item );
+            g->u.moves -= 100;
+            rc = it.type->id == new_item.type->id;


Whoa, when do you expect this to be false? player::i_add returns a reference to the added item in the inventory, it should never be a different item. But even if the item changed its type during pickup (why actually not?), that wouldn't mean the action failed (which is what rc == false would indicate).

I overlooked this comment, my bad. I wasn't sure what I wanted to do at the time, and figured making the id check was adequate, but adding an item shouldn't fail, so I removed it completely.

BevapDin · Jun 9, 2015

it.items.front() seems to be repeated quite often, why not move it into a nice convenient function.

The quoting style `...' makes me feel like reading a man page. Looks nice (-:

I tested this with a stack of newspaper pages. Moving them all with the "Move item stack" command works fine, as does "Move a single item", but moving a specific amount via "Move an amount of items" does not work: regardless of what I enter as amount to move, only one item is actually moved.

I get a crash when I try to pickup a specific amount of aspic from ground (amount being all there is, but using the "Move an amount of items" command). remove_item is called with the charges as amount_to_move, so it will try to 10 items from the ground, but the ground only has one item with 10 charges.

DavidKeaton · Jun 9, 2015

ah interesting! I shall look into it! I'm also a big fan of man :-)

DavidKeaton · Jun 9, 2015

I figured a simple get() would suffice in wrapping the it.items.front() up, yeah?
Testing the bugs you described as I type (compiling!)

DavidKeaton · Jun 11, 2015

been backing up my data, to migrate to a different dev box. sorry for the delay.

edit: back online ATM, sorry for the wait ya'll. (damn this box is so slow, gotta migrate this to a faster box soon o.O)

Coolthulhu · Jun 13, 2015

Moving part of a stack of by-charge items removes the entire stack and only adds the moved charges back.

Coolthulhu · Jun 13, 2015

Is move all from ALL supposed to work or not? First post seems to suggest that it should work, but it is still disabled.

DavidKeaton · Jun 13, 2015

move from all, not yet. I meant bringing up query_destination() and selecting a spot, now that spot is in vehicle storage. before it'd drop only on the ground.

DavidKeaton · Jun 13, 2015

I'll have to test the by_charges bit.

DavidKeaton · Jun 19, 2015

Sorry about the delay, more dev machine issues, should be migrating back to my original (or close to) soon.

Coolthulhu · Jun 19, 2015

Oops, I assigned without checking.

Will you be sending any changes in the next ~30 hours?

DavidKeaton · Jun 19, 2015

I should be. The machine works, per se, but it is difficult to do any real work on. I'm full wiping my normal box, but that won't take any time. (if I get some time, work all weekend).

Coolthulhu · Jun 19, 2015

Just realized I typed hours, while I wanted minutes.

I'll deassign for now. No rush - I just wanted to know if I'm supposed to wait for something and stay assigned (and thus warn other devs that I'm merging things).

If you're still working on it, it would be a good idea to edit the description and then change it back once you push your changes.

DavidKeaton · Jun 19, 2015

Ah good call, forgot to change the header a while ago. I'll change it back when it's ready. :-) Thanks bud!

DavidKeaton · Jun 26, 2015

Okay wow, dev machine should be good to go now. Sorry ya'll! Had a few issues.

DavidKeaton · Jul 27, 2015

Fixed

DavidKeaton · Jul 28, 2015

woo! thanks bud!

DavidKeaton force-pushed the DavidKeaton:aim branch Jun 12, 2015

DavidKeaton changed the title ~~AIM Updates~~ [RDY] AIM Updates Jun 12, 2015

Coolthulhu self-assigned this Jun 13, 2015

Coolthulhu assigned Coolthulhu and unassigned Coolthulhu Jun 13, 2015

Coolthulhu removed their assignment Jun 19, 2015

DavidKeaton changed the title ~~[RDY] AIM Updates~~ [WIP] AIM Updates Jun 19, 2015

DavidKeaton force-pushed the DavidKeaton:aim branch Jun 26, 2015

DavidKeaton force-pushed the DavidKeaton:aim branch to 8f73edd Jul 27, 2015

BevapDin self-assigned this Jul 28, 2015

BevapDin added a commit that referenced this pull request Jul 28, 2015

Merge pull request #12566 from DavidKeaton/aim
[RDY] AIM Updates

23ca6c9

BevapDin merged commit 23ca6c9 into CleverRaven:master Jul 28, 2015
1 check passed

1 check passed

default
Details

DanmakuDan referenced this pull request Nov 5, 2016
Merged
Fix advanced inventory and buckets #19178

CleverRaven/Cataclysm-DDA

Join GitHub today

[RDY] AIM Updates #12566

Conversation

DavidKeaton commented Jun 7, 2015

kevingranade reviewed Jun 7, 2015 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

molkemon commented Jun 7, 2015

BevapDin reviewed Jun 7, 2015 View changes

This comment has been minimized.

BevapDin reviewed Jun 7, 2015 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

BevapDin reviewed Jun 7, 2015 View changes

This comment has been minimized.

BevapDin reviewed Jun 7, 2015 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

DavidKeaton commented Jun 7, 2015

BevapDin reviewed Jun 9, 2015 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

BevapDin commented Jun 9, 2015

This comment has been minimized.

DavidKeaton commented Jun 9, 2015

This comment has been minimized.

DavidKeaton commented Jun 9, 2015

This comment has been minimized.

DavidKeaton commented Jun 11, 2015

DavidKeaton force-pushed the DavidKeaton:aim branch Jun 12, 2015

DavidKeaton changed the title AIM Updates [RDY] AIM Updates Jun 12, 2015

Coolthulhu self-assigned this Jun 13, 2015

This comment has been minimized.

Coolthulhu commented Jun 13, 2015

This comment has been minimized.

Coolthulhu commented Jun 13, 2015

This comment has been minimized.

DavidKeaton commented Jun 13, 2015

This comment has been minimized.

DavidKeaton commented Jun 13, 2015

Coolthulhu assigned Coolthulhu and unassigned Coolthulhu Jun 13, 2015

This comment has been minimized.

DavidKeaton commented Jun 19, 2015

This comment has been minimized.

Coolthulhu commented Jun 19, 2015

This comment has been minimized.

DavidKeaton commented Jun 19, 2015

This comment has been minimized.

Coolthulhu commented Jun 19, 2015

Coolthulhu removed their assignment Jun 19, 2015

This comment has been minimized.

DavidKeaton commented Jun 19, 2015

DavidKeaton changed the title [RDY] AIM Updates [WIP] AIM Updates Jun 19, 2015

DavidKeaton force-pushed the DavidKeaton:aim branch Jun 26, 2015

This comment has been minimized.

DavidKeaton commented Jun 26, 2015

DavidKeaton added some commits Jun 6, 2015

DavidKeaton force-pushed the DavidKeaton:aim branch to 8f73edd Jul 27, 2015

This comment has been minimized.

DavidKeaton commented Jul 27, 2015

BevapDin self-assigned this Jul 28, 2015

BevapDin added a commit that referenced this pull request Jul 28, 2015

Hide details View details BevapDin merged commit 23ca6c9 into CleverRaven:master Jul 28, 2015 1 check passed

1 check passed

This comment has been minimized.

DavidKeaton commented Jul 28, 2015

DanmakuDan referenced this pull request Nov 5, 2016

kevingranade reviewed Jun 7, 2015

View changes

BevapDin reviewed Jun 7, 2015

View changes

BevapDin reviewed Jun 7, 2015

View changes

BevapDin reviewed Jun 7, 2015

View changes

BevapDin reviewed Jun 7, 2015

View changes

BevapDin reviewed Jun 9, 2015

View changes

DavidKeaton force-pushed the DavidKeaton:aim branch to `8f73edd` Jul 27, 2015

BevapDin merged commit `23ca6c9` into CleverRaven:master Jul 28, 2015
1 check passed