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

Stop trimming '!' and '?' from tactic name. Fixes #299 #304

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tautastic
Copy link

Stop trimming '!' and '?' from tactic name. Fixes #299. I have used main as the base branch since I don't know how to test on dev since the editor is not working on dev.

@joneugster
Copy link
Collaborator

That would be the way to do this, unless one would actually work with fully-qualified tactic names, in which case one could use the stuff underlying @[tactic_alt].

Also, it's probably worth thinking for a second if there are any cases where having something like simp? around would actually be desirable.

Anyways, I would like to wait with this for a moment, until dev is a bit more ready.

@TentativeConvert
Copy link
Collaborator

What would the fix proposed here actually do?

I’d say the ideal behaviour would be that simp? and apply? are treated like any other tactic, i.e. that it is left to the game developer to decide whether to make them available and/or visible or not, and that the game developer can make this choice independent of their choice for the cousins simp and apply. I fully agree that there may be cases where having simp? around would be desirable.

@joneugster
Copy link
Collaborator

True, and since we do have NewHiddenTactic for a while now, there is actually a way to add simp? without polluting the inventory 👍

@tautastic
Copy link
Author

What would the fix proposed here actually do?

The proposed fix treats apply, apply? and apply! all as distinct tactics. You can enable or disable each one seperatly, and like Jon said you can enable apply? and apply! with NewHiddenTactic, this would result in the same user experience as before (you see only apply in your inventory but you can use apply? and apply!)

@TentativeConvert
Copy link
Collaborator

Thanks for clarifying! (I’ll leave it to @joneugster to merge when dev is ready.)

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.

Can we distinguish apply? from apply
3 participants