CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.64k stars 4.18k forks source link

Trying to use invalid color tags causes a CTD without a useful error message #77747

Open Procyonae opened 2 days ago

Procyonae commented 2 days ago

Describe the bug

get_color_from_tag() eventually calls color_from_string() which explicitly supports tags but trying to use such tags like the examples below results in an error which when dismissed causes a CTD

Confusingly we support 8 foreground colours and 8 background colours but only bold foreground colours not bold background colours which makes it easy to use invalid colour tags such as

Image showing the available fg/bg combos: image

This could be fixed by either making the remaining colour combos valid but this is a technical limitation of some older terminals esotericist: "it IS a thing we COULD consider trying to change, since the number of terminals who can't do bold background colors in service is a lot lower, but it's also kinda a diminishing returns territory to implement just for ascii art, and i think it'd be a usability error if we also added it for general use (we have enough trouble with bad color combos as it is, bold background colors complicates all of that even more)" or alternately just provide a proper error message and update the colour docs to be more transparent about what does and doesn't work

Attach save file

N/A

Steps to reproduce

  1. Add a tag like (and closing tag) to an existing "description" or "ascii_art" (I were using "battery_car")
  2. Try to examine "battery_car" in game
  3. Error prompt
  4. Pick your poison
  5. CTD

Expected behavior

Pretty stuff :c

Screenshots

image Abort or Retry results in CTD, Ignore results in a new identical error prompt

Versions and configuration

Additional context

Boondiggle on the devcord was having problems with REXPaint's CDDA export function so I figured I'd write a python script that converts its save format straight to a complete json object and runs the formatter on it so we don't have to rely on the closed source being kept up to date with any changes to structure we require so I can provide a good test ascii art with every fg/bg combo for any PR solving this or unit test we add to stop this breaking again.

Thanks Boondiggle, Lumi Virtual and esotericist for figuring stuff out for this on devcord, updated in light of their info

mqrause commented 2 days ago

I didn't track down why exactly the crash happens, but it looks like color_light_cyan_light_blue is missing in color_manager::load_default. And other combinations might be missing as well.

mqrause commented 1 day ago

I think the crash is related to debug messages showing in the middle of imgui drawing, so not directly related to the specific error. See #77763 for a different but similar case.

db48x commented 13 hours ago

Yes, anything that causes an error while in the middle of an ImGui window is just going to cause a second error from ImGui itself, which is super annoying. I think there was already a bug filed specifically about that, but I don’t recall the title or the number offhand.