|Anonymous | Login | Signup for a new account||2019-02-23 23:05 CET|
|Main | My View | View Issues | Change Log | Wiki | Tavern | News|
|Viewing Issue Simple Details|
|ID||Category||Severity||Reproducibility||Date Submitted||Last Update|
|0007789||[DCSS] Patches||feature||N/A||2013-11-27 22:00||2013-11-30 17:23|
|Status||closed||Product Branch||0.14 ancient branch|
|Summary||0007789: Display number of affected monsters for Zin's recite|
Recite against which type of sinner?
[a] - Chaotic (affects the forces of chaos and mutation)
2: a green ugly thing, a white very ugly thing
[b] - Heretic (affects non-believers)
7: an orc, an orc sorcerer, an orc priest, an orc warrior, an orc, an orc...
Initially I tried to put monster list to the right of the same row but unfortunately there is very little room: "1: ..." is displayed instead of "1: a white very ugly thing".
If there are more than 2 types of sinner, "more" prompt can trigger.
|Tags||No tags attached.|
|Attached Files||recite.patch [^] (1,733 bytes) 2013-11-27 22:00 [Show Content]|
edited on: 2013-11-30 00:37
I like the idea; several comments on the patch:
First, the display is a little ugly with the text unindented. What about, as dpeg suggested in Tavern, removing the parenthetical description. Then you could let the list take up to two lines, and keep the second line indented. Something like:
Recite against which type of sinner? [a] - Chaotic (2: a green ugly thing, a white very ugly thing) [b] - Heretic (7: an orc, an orc sorcerer, an orc priest, an orc warrior, an orc, an orc)
The repeated "an orc" is kind of ugly. Perhaps identical names could be collapsed into something like "an orc x3"? Doing genus factoring (as in come-into-view messages) might be a bit too much, as it would give just "7 orcs".
Also, use strwidth(s.c_str()) rather than s.length(), or it won't handle unicode properly: length() returns the number of bytes, which these days has little to do with how much space it takes to print. It's not so much of an issue now (no monsters have non-ASCII names), but will be when we implement more translation.
It would be nice to not hardcode a width of 80, so tiles and bigterm players don't have wasted space. Unfortunately there's not a simple way to get the message window width. msgwin_line_length() would do it, but it would need to be made non-static and added to a header.
Finally, please use spaces instead of tabs; checkwhite can do the conversion, but I don't think it handles your four-space tabs properly, so it would be better to configure your editor to do it for you if possible.
|Sorry, I am not a C++ programmer so the code can be very ugly, for example I used google to find how C++ arrays are created and string length is detemined. Please let me know if you want me to continue working on the patch, I am not sure how much time it can take.|
|No worries, I'll work on the cleanups I mentioned.|
|Applied in trunk (0.14-a0-1196-g2214da5), with some changes to the formatting as discussed here (up to and including 0.14-a0-1198-g82f56c9). Thanks!|
|How should we credit you?|
|No credit is needed, it was my pleasure to do something useful for this great game.|
|2013-11-27 22:00||Sandman25||New Issue|
|2013-11-27 22:00||Sandman25||File Added: recite.patch|
|2013-11-30 00:34||neil||Note Added: 0024537|
|2013-11-30 00:37||neil||Note Edited: 0024537|
|2013-11-30 03:03||Sandman25||Note Added: 0024545|
|2013-11-30 05:05||neil||Status||new => assigned|
|2013-11-30 05:05||neil||Assigned To||=> neil|
|2013-11-30 05:05||neil||Note Added: 0024551|
|2013-11-30 17:08||neil||Note Added: 0024552|
|2013-11-30 17:08||neil||Status||assigned => resolved|
|2013-11-30 17:08||neil||Fixed in Branch||=> 0.14 development branch|
|2013-11-30 17:08||neil||Resolution||open => done|
|2013-11-30 17:09||neil||Note Added: 0024553|
|2013-11-30 17:21||Sandman25||Note Added: 0024554|
|2013-11-30 17:23||Sandman25||Status||resolved => closed|
|Mantis 1.1.8[^] Copyright © 2000 - 2009 Mantis Group|