Skip to content

Conversation

@NQNStudios
Copy link
Collaborator

@NQNStudios NQNStudios commented Aug 12, 2025

I went through the big PR and cherry-picked all the bugfixes that apply to master.

For 51ae2e7, an alternate solution is possible where we would just discard empty area description rects when we load a legacy scenario. Every area seems to start with Rectangle 1 - Rectangle 8 as empty rectangles in there at (0,0), so in master, if you stand at 0,0 of an outdoor section the status bar will say you're in Rectangle 8. One reason not to discard them is that you can edit their text without initializing their bounds, so maybe some legacy scenario has text on one of those rectangles that would get dropped. Probably wouldn't matter at all, but idk.... Discarding them would be nice because my map overhaul code (not in this PR) also has to check for and skip empty rectangles in two places.

In 0479c3e at the end, I've started making it so the Edit Specials list gives you more info on each node than just the type name. This is really gonna help with debugging old scenarios.

Copy link
Member

@CelticMinstrel CelticMinstrel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This more or less looks good to me – I can't see anything particularly controversial. I'm not 100% sure about everything in here but there's also nothing I'm particularly concerned about.

bool try_move(short i,location start,short x,short y);
bool combat_move_monster(short which,location destination);
location find_clear_spot(location from_where,short mode);
location find_clear_spot(location from_where,short mode,short x_width = 1, short y_width = 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the default values actually needed? If we actually specify the value in every call site, I think it's better to remove them…

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a call site where I used the defaults (summoning logic, where big monsters are disallowed.)

@NQNStudios NQNStudios force-pushed the picks1 branch 4 times, most recently from 99c6e33 to 07149c9 Compare August 16, 2025 19:20
@NQNStudios
Copy link
Collaborator Author

@CelticMinstrel two new commits today reflect something fun I learned: the fonts we've been using have always had the characters we need. Substitutions weren't necessary, we just needed to tell SFML our text was UTF-8.

@CelticMinstrel
Copy link
Member

Oh, nice! I think I'll merge this soon, then – I would actually like to merge my fmtlib PR first though.

Also, I just looked into the system surrounding 07149c9 a bit more, and I get the feeling that the require_charges parameter actually isn't really needed; but as long as it does exist, your change seems reasonable.

Currently, require_charges is always false when just testing if a PC has an item of the class, but it's true when the item is supposed to be taken. So, the take_class function could probably be reworked to handle the case of a 0-charge wand (if that is indeed possible) without needing the extra parameter in has_class.

@NQNStudios
Copy link
Collaborator Author

Xcode build is throwing an error that comes from an unannotated fallthrough in SFML code 😭

@NQNStudios NQNStudios force-pushed the picks1 branch 2 times, most recently from f843f89 to 4b74d1d Compare August 18, 2025 22:29
@NQNStudios
Copy link
Collaborator Author

I was wrong, our plain font doesn't have an emdash, so I'm bringing back substitutions for now (just with that one).

@NQNStudios
Copy link
Collaborator Author

I was wrong AGAIN, the emdash is fine, but the scenario editor can't type an emdash correctly in a text field on mac.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants