Attempting to open crafting menu causes crash, if window width is too small

[chaosvolt]

I found this after testing the aftermath of a PR ( #14221 ), but I would assume that PR was not the cause.

Initial attempt was a trapper who walked outside. Second attempt, wound up as a backpacker and stayed indoors. Also failed. Unless we're having a repeat of the "long rope, shoulder strap, whoops here have a freeze" bug with some item common to both professions, this is likely a general issue.

Windows, tiles build 3980.

[remyroy]

I can confirm this on Windows tiles build 3981. This should be a priority.

[remyroy]

I just tested this on Ubuntu and I cannot reproduce the problem.

[chaosvolt]

Hmm. I'm guessing it's a Windows-specific issue? What about tiles versus curses build?

[DanmakuDan]

The w_iteminfo window for the crafting menu is trying to create a sub-window with width -1, because the window is too narrow at the default 80 width for whatever it's doing. It's more likely to happen in a tiles build.

[remyroy]

I just tested this on Mac OS 10.9 and here is the error dump I got: https://gist.github.com/remyroy/c1753e572bae3a31d52d

[chaosvolt]

*facepalms*

[remyroy]

I can reproduce this on Ubuntu with the default terminal size (80x24).

[chaosvolt]

Testing if it works with my preferred window setting of 100x40. Between this and the minimap being squashed to the point of uselessness, I'm wondering whether the default should be bumped up a bit.

EDIT: It works. Yaaay.

[DanmakuDan]

HIstorical terminal sizes are 80x(24/25), which is what the game interface was built around.

[chaosvolt]

That does explain it, yes. Oh well. Granted, so long as this issue is solvable, then there's no real need to bump up the defaults.

The minimap flattening is not an issue, and if anything would be an unsolvable one. The smaller the window size, the less of the UI that can be efficiently displayed.

[remyroy]

w_iteminfo should probably be relocated or only shown when there is plenty of width space to be meaningful. What do you think @OzoneH3 ?

[remyroy]

This is related to #14158.

[chaosvolt]

This new window is never deleted.

Why does that comment fill me with a strange sense of dread? ;w;

[remyroy]

A workaround for this issue is to set your terminal width to at least 96. This seems to be the minimum value at which it won't crash.

[DanmakuDan]

Why does that comment fill me with a strange sense of dread? ;w;

It only means it causes a memory leak when the crafting menu is accessed. Something small like that would take a long time to be noticeable.

What really happened is that w_iteminfo used to be part of w_data, and if the code saw that the window was wide enough, it would use the rest of w_data to print the description of the item. Now that the ingredient window and item description window are split, there would have to a be a separate consideration for the w_iteminfo window being applicable for different game window widths.

[chaosvolt]

Which is good given the setting I've gotten used to is a tad wider than that.

Do we have a "setting-dependent" label for situations like this? :V

And...ah, I see.

[chaosvolt]

The amusing thing is I would normally consider it a bad habit to test things like the flicker fix with config settings that are different from what one would actually use during gameplay, but if I wouldn't have discovered this issue if I hadn't started testing in default settings.

[drbig]

Hmm, ok, not limited to Windows. Perhaps the minimum terminal size should be changed.

[chaosvolt]

Hmm. It'd be nice if we had some way to survey what terminal sizes are the most-used by players. Or we could just bump the defaults up by the bare minimum to avoid this crash, while retaining the same ratio as the current default.

So if the width has to be at least 96, that'd be...28.8? Hnng. 100x30 might be less messy while retaining the ratio exactly.

[remyroy]

There is no need to change the minimum terminal width. This is simply a UI issue for which there are various possible solutions.

Resizing the other windows when there is not enough room for w_iteminfo so that w_iteminfo can have enough space is one possible solution. Hiding w_iteminfo when there is not enough space for it is another solution. Relocating w_iteminfo to be below the recipe requirements when there is not enough space to the right is another solution. They have all various pros and cons.

[chaosvolt]

Probably best to fix the issue at hand rather than settle for a workaround, yeah. Regardless of whether or not the idea worth considering, that'd be the subject of a separate suggest issue or PR.

[drbig]

It'd be nice if we had some way to survey what terminal sizes are the most-used by players.

The survey on starting points did better than expected. Getting actual feedback is the best way to decide.

They have all various pros and cons.

Indeed.

While I'm all for more "dynamic" (and certainly more consistent!) UI having certain basic assumptions about size doesn't sound bad to me either (we can do both).

[chaosvolt]

The survey on starting points did better than expected. Getting actual feedback is the best way to decide.

And even if it isn't used for anything, it's interesting to have.