Skip to content
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] update quickfort library aliases #1760

Merged
merged 12 commits into from Jan 31, 2021

Conversation

myk002
Copy link
Member

@myk002 myk002 commented Jan 24, 2021

discovered by ldog on the forums

fixed aliases for tallow, iron, copper, and steel weapons; added aliases for bronze and crafts

@myk002 myk002 changed the title fix quickfort library aliases for weapons [quickfort] fix quickfort library aliases for weapons Jan 24, 2021
@myk002 myk002 changed the title [quickfort] fix quickfort library aliases for weapons [quickfort] update quickfort library aliases Jan 27, 2021
@@ -104,7 +104,7 @@ plants: {foodprefix}b{Right}{Down 4}p^
booze: {foodprefix}b{Right}{Down 5}p{Down}p^
seeds: {foodprefix}b{Right}{Down 9}p^
dye: {foodprefix}b{Right}{Down 11}{Right}{Down 28}{togglesequence 4}^
tallow: {foodprefix}b{Right}{Down 13}{Right}{Down}{togglesequence2 811}^
tallow: {foodprefix}b{Right}{Down 13}{Right}{Down}{togglesequence2 822}^
Copy link
Member

Choose a reason for hiding this comment

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

This feels like something that could be highly dependent on raws to me - have you verified that this works with vanilla raws now?

This is one of those cases that makes me think some special logic for certain screens could be a good idea, to make sure that aliases like these work for people with mods. In this case, the screen has an item_names vector (which the search plugin uses, incidentally) that could be easy to search.

I realize this is way out of the scope of this PR, and keeping compatibility with existing blueprints would be important, but might be worth spinning out into a separate issue (if there's not already one I'm forgetting)

Copy link
Member

Choose a reason for hiding this comment

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

Update: this actually depends on how many generated creatures there are. I have a save with 815*2 entries in the "Fat" list. so 822 would wrap around and re-enable some fat entries at the top. Raws themselves are vanilla.

Not an easy screenshot to read, but here's what happens with 822:
image

811 leaves some entries on at the bottom:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

that would explain why the number was off. I was wondering how in the world could I have gotten it wrong when I wrote it the first time.

so I see three options here:

  1. use the minimum number of creature types (0 generated types, just vanilla raws) and just say that's "good enough" -- how often do generated creature types show up on fortress maps? This if course can fall flat if a modded game has fewer creature types...
  2. use the search plugin. now that we've solved the disappearing prompt bug (Search prompt doesn't appear the second time you enter a screen #1725), this is feasible. I was hoping to avoid any hard dependencies on other plugins (even buildingplan is optional), but search is enabled by default so it is a relatively safe dependency (right?)
  3. implement "alias functions" that are invoked like aliases but run software functions instead of just expand into static keycode sequences. I was intending to do this anyway for features like "assign minecart to hauling route", but I have a lot of open questions about the design of this feature, and I'm not sure if I want to commit myself to a particular implementation yet.

so unless the search plugin isn't as safe a dependency as I think it is, I think I'll go with option 2 in the short term and maybe convert to option 3 in the long term.

Copy link
Member Author

Choose a reason for hiding this comment

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

I implemented option 2. Do you think using search is sufficiently reliable (that is, are there any classes of users who don't have the plugin enabled by default?)

Where is the best place to document the dependency? I mentioned it in the alias guide, but that is targeted at blueprint creators. Where will regular users see it? It seems slightly out of place in the command reference, but if there is no place better, I can add it there

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what failed on buildmaster.. I don't see any error messages, and each of the individual builds seems to have succeeded.

Copy link
Member

Choose a reason for hiding this comment

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

Ooh, using p and f is smart! I didn't remember that would work. I think (3) is what I was leaning towards in my first comment as well, but this is a good shorter-term solution.

As for docs, maybe something along the lines of "some stockpile functionality currently requires the search plugin to be enabled" in more user-facing docs, then details in the alias guide could work.

I only see 5/8 of the expected build outputs at https://buildmaster.lubar.me/applications/17/builds/build?buildId=9219 - Buildmaster went down for maintenance around when you commented, so that may be related. (In fact, I think this PR might have been the built that hung for a couple hours and triggered said maintenance.) I don't see any issues, so I'll ignore it.

and document the dependency
@lethosor lethosor added this to In progress in 0.47.04-r5 via automation Jan 30, 2021
@lethosor lethosor merged commit 472f19e into DFHack:develop Jan 31, 2021
0.47.04-r5 automation moved this from In progress to Done Jan 31, 2021
@myk002 myk002 deleted the myk_alias_fix branch January 31, 2021 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
0.47.04-r5
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants