-
Notifications
You must be signed in to change notification settings - Fork 193
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
Quickfort: implement dig priorities #190
Conversation
instead of dereferencing a nil pointer and crashing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I found an issue where designations with no priority are assigned a priority of 0, not 4, effectively giving them the highest priority (I think this is something that another tool changing designation priorities has run into before). Here's a test blueprint:
#dig
1,1,1
d,d,d
2,2,2
I'm expecting the middle row to be dug out last, but it's dug out first (followed by the top row, then the bottom).
A more complicated one, which seems to exhibit the same behavior of non-prioritized tiles being dug out first:
#dig
d,d4,d5
1,2,h3
4,d4,d
return custom_designate | ||
end | ||
|
||
setmetatable(dig_db, {__index=extended_parser}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, extended_parser will never get called for keys that are already in dig_db
(like "d" from my examples).
I'm not sure if you accounted for this - it looks like extended_parser is intended to handle keys without numbers, which I don't think it can be called with.
Update: I see set_priority() guarded by a check for dig_db[key].use_priority, but it doesn't seem to be called when the priority isn't specified, and it probably should be called with 4. I'm assuming you have a better idea of how to address this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it still gets called when there is an 'm' prefix for marker mode, so "md" would get here (and assign the default priority of 4).
I wasn't calling set_priority() when priority isn't specified since the game interprets lack of an event block as all priorities == 4, and I didn't think I needed to mess with them. but in this case what might be happening is an event block already exists with a 0 value and I'm not overwriting it? It's true that I hadn't considered that case. it could happen with non-zero values as well, I think. if a previous designation was at priority N, was cleared (which does not clear the event block, IIRC), and then a blueprint without an explicit priority was applied. I think I can fix this by assigning priority=4 to the db records.
in your case, though, I wonder how a priority of 0 ever got set.. I didn't think that was possible.
edit oooooh I understand now. I created the event block when I encountered the other tiles with a priority set. that creates a map that is initialized to 0. the 'd' tiles didn't set any priority, so the 0 value never got changed for those tiles. ok, cool. yes, setting priority=4 in the db should fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, block events cover the whole block. You're not the first to run into issues caused by this. Looks good now!
internal/quickfort/dig.lua
Outdated
local pbse = get_priority_block_square_event(block_events) | ||
if not pbse then | ||
if priority == 4 then return end | ||
print("creating event") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should be a log()
call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was a leftover dev debugging line; removed.
implement setting the priority for dig designations. syntax for dig blueprints is now [letter][number] where if the letter is not specified, 'd' is assumed, and if number is not specified, 4 is assumed.
also fix a bug in has_designation where the uint32 values were interpreted as booleans instead of numbers when they were unset. changed code to be unambiguous.
edit: also fix a bug in my new fancy __index methods where we could overflow the stack if we tried to find a key that doesn't exist.