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

Advanced inventory 'm'ove autofills dialogue #20234

Closed
sick-trigger opened this issue Feb 9, 2017 · 7 comments

Comments

4 participants
@sick-trigger
Copy link
Contributor

commented Feb 9, 2017

Using 'm' for moving part of a stack in the advanced inventory now automatically fills the dialogue box with the full size of the stack, when it used to start empty. Means you now have to backspace every time you use it. If you wanted to move the whole stack you can just use upper-case 'M' instead.

possibly related: when searching in the wish-for-item menu the cursor now always starts at the beginning, even if the box already has something in it from a previous search, when it used to start at the end of any text. Means you now have to move the cursor back to the right or END when spawning different types of items.

both present in build 6217, but not 6208 (latest previous successful build). Possibly from #20033?

@keyspace

This comment has been minimized.

Copy link
Contributor

commented Feb 10, 2017

Confirming both issues, haven't bisected.

Arch Linux, curses, self-built, lastest latest git master (0.C-21086-g3aea92e4fd).

@keyspace

This comment has been minimized.

Copy link
Contributor

commented Feb 10, 2017

Doing a bisect, confirm PR #20033 as culprit (EDIT4: for both issues outlined above, and the one below).

Bisect is skip-stuck (doesn't compile without modification) on commits in the PR: here's the log.

Noticed another issue: when moving part of stack with m, pressing ESC still moves the whole stack, even if a smaller number was entered. Expected behaviour is to cancel moving.

EDIT2: The latter could be related to the overarching "global binds" regression, as described in #20065, or any issues linked from there.

EDIT3: Didn't open a separate issue due to it being the same menu, and caused by same PR. Can do so if to be addressed separately.

Ping @BevapDin :(


EDIT: typo

@keyspace

This comment has been minimized.

Copy link
Contributor

commented Feb 10, 2017

"Amount auto-filled" can possibly be corrected on this line.

"ESC moving instead of cancelling" - a few lines above, but requires reviewing function's logic. EDIT: Perhaps check for popup cancelled, like here?..

EDIT2: Debug menus are not using input context et al., so probably best addressed separately after all.


Note: AIM m move does place the input cursor at the end, different from wish-item search - after all, the latter is not a popup.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Mar 8, 2017

Both issues still present after a number of fixes.

@kevingranade kevingranade added this to the 0.D milestone Mar 8, 2017

@kolsurma

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2017

Haven't touched this particular issue.

What is the desired default behavior for the move-amount: an empty prompt or 0?

@keyspace

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2017

@kolsurma it used to start empty, which seems desirable to me.

When m is pressed to move a custom-sized stack, pre-inserting a 0 in would mean that it'd have to be erased anyway.

@kolsurma

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2017

The reason I asked was because the offending code passes the amount by reference into
void string_input_popup::edit( long &value ) and long can't be empty so...

The easy thing to do there would be to remove text( to_string( value ) ); or change which function it calls. At the moment, advanced_inventory::query_charges is the only thing that calls this function. I'll try the former first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.