-
Notifications
You must be signed in to change notification settings - Fork 191
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 place #168
Quickfort place #168
Conversation
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.
Cross-linking the forum post for reference. Looks good!
I assume you have test blueprints of some sort locally? No rush to get those up, but it would be nice to have some to play around/test with eventually. I mostly just looked through the code here.
re:test blueprints -- I haven't created a formal suite yet. I did a bunch of manual one-offs to test in different terrain and boundary conditions. Once the structure of the code settles down and basic functionality is complete, I plan to put together unit tests that exercise all the conditions. Once we start adding advanced features, I really want to have a regression testing suite in place to make sure nothing gets broken. In the meantime, I have a sample blueprint up at https://docs.google.com/spreadsheets/d/1KcNFC63m-iHLPPdttosR5cuNiZ0vUtxlT51T_MV0588/edit#gid=706490003 Thank you for all these code reviews, by the way. I know this is a huge amount of code very quickly, and I appreciate the feedback that you give. |
BTW, I left stockpile settings initialization as a TODO. I'll come back to that after I refactor parts of the current stockpile plugin logic into a stockpile module. |
That seems to work, thanks! (I didn't notice the |
DFHack/dfhack#499 Implement stockpile placing logic for the quickfort script
Enforces the same restrictions and behavior as performing the actions from the UI
This PR does not implement stockpile setting initialization. That will take some refactoring of the current stockpile plugin logic into a stockpile module. I'll address that later.