-
-
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
Feature: Architect Notifications when puzzle fails #1197
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.
I feel this feature should be a sepearate feature outside of the dungeon co-pilot as afaik this feature is not fully finished and impacts performance a lot, I'll do further reviews once this is changed
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 still would like to see this fully separated from dungeon copilot, not needing that to be enabled to use this and that not being the pattern key.
src/main/java/at/hannibal2/skyhanni/features/dungeon/DungeonCopilot.kt
Outdated
Show resolved
Hide resolved
src/main/java/at/hannibal2/skyhanni/config/features/dungeon/DungeonConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/at/hannibal2/skyhanni/features/dungeon/DungeonArchitectFeatures.kt
Outdated
Show resolved
Hide resolved
…ngeonConfig.java Co-authored-by: CalMWolfs <94038482+CalMWolfs@users.noreply.github.com>
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.
Haven't tested in game but I fixed the remaining issues with the code
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.
Im not a fan of the repopattern key names. the dungeon chat messages should start with dungeon.chat.xyz, and not contain the feature name. we might add new features in the future that require the same chat messages, then the name is weird.
also for normal.puzzle.fail
and quiz.puzzle.fail
:
please rename to puzzle.fail.normal and puzzle fail.quiz
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.
Please include the information what puzzle is failed, or disable this feature per default.
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.
This pr is fine. Adding the name of the puzzle is not necessary in my opinion since you could look that up via tab or simply know because of what player failed.
What
Sends a title screen and a chat notification overwriting the one for puzzle fail, reminding the user to use an architect, and provides a button that can be clicked to retrieve it from sacks.
Demonstration
https://conutik.catto.pics/748fad3bChangelog New Features
Changelog Technical Details
RepoPattern
with good key naming conventions.