Conversation
Add the standalone daily quest module, task planner, native quest sources, combat helper, and PET NPC locator.
Cover daily detection, nested requirements, NPC and map matching, reward filtering, and Uridium acceptance.
Describe daily quest automation, Uridium filtering, PET shutdown, and configuration isolation.
Contributor
Reviewer's GuideImplements a standalone DailyTaskAPI module for discovering and completing daily quests, adds a PET locator integration, and wires the new features into the plugin manifest and documentation without touching other modules’ settings. Sequence diagram for DailyTaskAPI quest run and PET locatorsequenceDiagram
actor Player
participant BotAPI as Bot
participant DailyTaskAPI as Daily
participant QuestAPI as Quests
participant DailyNpcCombat as Combat
participant PetAPI as Pet
participant DailyNpcPetLocator as PetLocator
Player->>Bot: start bot with DailyTaskAPI
Bot->>Daily: onTickModule()
Daily->>Quests: getDisplayedQuest()
Daily->>DailyTaskPlanner: actionable(quest)
Daily->>DailyNpcCombat: tick(targetNpcDescription)
DailyNpcCombat-->>Daily: combat status
loop quest progress
Daily->>Quests: progress / completion
Daily-->>Bot: getStatus()
end
note over PetLocator,Pet: PET gear & locator selection
Pet->>PetLocator: getGearSupplier()
PetLocator->>Bot: getModule()/getNonTemporalModule()
Bot-->>PetLocator: DailyTaskAPI instance
PetLocator->>Daily: getNpcLocatorTargetDescription()
Daily-->>PetLocator: NPC description
PetLocator->>Pet: PetGear.ENEMY_LOCATOR
Pet->>PetLocator: getNpcLocatorPick(available)
PetLocator-->>Pet: pick matching NPC
Daily->>Bot: finishAll() when all dailies done
Daily->>Pet: stopCombat(), disable PET
Daily-->>Bot: setRunning(false) (if enabled in config)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- DailyTaskConfig.maxMenuScanPasses is defined but never used; either integrate it into the quest menu scanning logic or remove it to avoid dead configuration.
- DailyNpcCombat.stopCombat currently disables the PET unconditionally; consider checking and restoring the user's previous pet-enabled state rather than forcing it off.
- DailyTaskAPI’s PET deactivation logic relies on reflective access to a private 'pet' field in DailyNpcCombat; this is brittle against internal refactors and could be replaced with a more explicit API or a dedicated callback for PET shutdown.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- DailyTaskConfig.maxMenuScanPasses is defined but never used; either integrate it into the quest menu scanning logic or remove it to avoid dead configuration.
- DailyNpcCombat.stopCombat currently disables the PET unconditionally; consider checking and restoring the user's previous pet-enabled state rather than forcing it off.
- DailyTaskAPI’s PET deactivation logic relies on reflective access to a private 'pet' field in DailyNpcCombat; this is brittle against internal refactors and could be replaced with a more explicit API or a dedicated callback for PET shutdown.
## Individual Comments
### Comment 1
<location path="src/main/java/dev/shared/berke/dailytaskapi/DailyTaskAPI.java" line_range="130" />
<code_context>
+ private final Set<Integer> reviewedOfferIds = new LinkedHashSet<>();
+ private final Set<Integer> seenGiverOfferIds = new LinkedHashSet<>();
+ private final Set<Integer> acceptedOfferIds = new LinkedHashSet<>();
+ private final Set<Integer> skippedTetrathrinOfferIds = new LinkedHashSet<>();
+
+ public DailyTaskAPI(PluginAPI api) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Tetrathrin-only reward skipping is not actually implemented, leading to misleading status and unused tracking.
`skippedTetrathrinOfferIds` is declared and “Tetrathrin pas” is shown in `finishQuestOfferScan`, but the set is never updated and `DailyTaskPlanner.isOnlyTetrathrinReward` is never invoked. This means Tetrathrin-only offers are not actually skipped and the status will always show 0 passes. Please either hook this logic up in `inspectGiverRow` (populate `skippedTetrathrinOfferIds` and skip those offers) or remove the unused set and status to avoid misleading output.
</issue_to_address>
### Comment 2
<location path="src/main/java/dev/shared/berke/dailytaskapi/DailyTaskConfig.java" line_range="25" />
<code_context>
+ @Number(min = 1, max = 5, step = 1)
+ public int maxMenuScanPasses = 1;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Configuration fields `maxMenuScanPasses` and `stopWhenAllDailyTasksAreDone` are never honored by the module.
These two config options are defined but never influence `DailyTaskAPI` behavior: `menuScanPass` is never checked against `maxMenuScanPasses`, and `finishAll()` always sets state to `DONE` and stops the bot regardless of `stopWhenAllDailyTasksAreDone`. Please either wire these fields into the logic or remove them from the config to avoid misleading, unused settings.
</issue_to_address>
### Comment 3
<location path="src/main/java/dev/shared/berke/dailytaskapi/DailyNpcCombat.java" line_range="27" />
<code_context>
+ super(api);
+ }
+
+ void tick(String description) {
+ targetDescription = description;
+ ConfigSetting<Boolean> petEnabled = api.requireAPI(ConfigAPI.class).requireConfig("pet.enabled");
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Force-enabling PET via config and then disabling it on stop may conflict with user settings and other modules.
`tick` forces `pet.enabled` to `true` on every call, and `stopCombat` turns it off without regard for the previous value. This overrides the user’s global PET preference and can conflict with other modules using `pet.enabled`. Please preserve and restore the original setting or delegate PET control to `DailyNpcPetLocator`/the PET manager instead of mutating the shared config directly.
Suggested implementation:
```java
final class DailyNpcCombat extends LootModule {
private String targetDescription;
private Boolean originalPetEnabled;
```
```java
void tick(String description) {
targetDescription = description;
// Capture the original PET enabled state once, then delegate control to the PET manager
if (originalPetEnabled == null) {
originalPetEnabled = pet.isEnabled();
}
if (!pet.isEnabled()) {
pet.setEnabled(true);
}
super.onTickModule();
}
```
```java
void stopCombat() {
attack.stopAttack();
attack.setTarget(null);
targetDescription = null;
// Restore PET state to whatever it was before this module started manipulating it
if (originalPetEnabled != null && pet.isEnabled() != originalPetEnabled) {
pet.setEnabled(originalPetEnabled);
}
originalPetEnabled = null;
}
```
This change assumes:
1. The `pet` field is a PET manager/DailyNpcPetLocator that exposes `isEnabled()` and `setEnabled(boolean)` and internally respects the global `pet.enabled` config.
2. `tick` is only called while this module is active; if other modules also toggle PET, you may need a more sophisticated ownership/priority mechanism in the PET manager to avoid conflicts.
3. If PET can be `null` in this class, wrap all `pet` accesses in null checks.
If other parts of the file cached or manipulated `pet.enabled` via `ConfigAPI`, those should be updated to use the PET manager as well to keep all PET control centralized.
</issue_to_address>
### Comment 4
<location path="src/test/java/dev/shared/berke/dailytaskapi/DailyTaskPlannerTest.java" line_range="77-80" />
<code_context>
+ "a quest containing Uridium must be accepted even with Tetrathrin");
+ require(!DailyTaskPlanner.isOnlyTetrathrinReward(List.of(reward("currency_experience", 1000))),
+ "currency-only rewards are not a Tetrathrin variant");
+ require(DailyTaskPlanner.hasUridiumReward(List.of(
+ reward("currency_experience", 1000), reward("currency_uridium", 1500))),
+ "Uridium reward must be detected from the source reward type");
+ require(!DailyTaskPlanner.hasUridiumReward(List.of(reward("resource_tetrathrin", 10))),
+ "a reward list without Uridium must be rejected");
+ System.out.println("DailyTaskPlannerTest: OK");
</code_context>
<issue_to_address>
**suggestion (testing):** Missing tests for `progress(...)` and timer-related requirement handling in the planner
The planner’s `progress(...)` and `actionableIncludingCompleted(...)` logic (zero-goal requirements, completed vs. non-completed, and exclusion of haste/timer types) is currently untested. Please add tests that cover quests with timer requirements, zero-goal requirements, and partially completed requirements, and assert the resulting progress values so we verify the daily completion detection and 99.99% threshold behavior used by `DailyTaskAPI`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| private final Set<Integer> reviewedOfferIds = new LinkedHashSet<>(); | ||
| private final Set<Integer> seenGiverOfferIds = new LinkedHashSet<>(); | ||
| private final Set<Integer> acceptedOfferIds = new LinkedHashSet<>(); | ||
| private final Set<Integer> skippedTetrathrinOfferIds = new LinkedHashSet<>(); |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Tetrathrin-only reward skipping is not actually implemented, leading to misleading status and unused tracking.
skippedTetrathrinOfferIds is declared and “Tetrathrin pas” is shown in finishQuestOfferScan, but the set is never updated and DailyTaskPlanner.isOnlyTetrathrinReward is never invoked. This means Tetrathrin-only offers are not actually skipped and the status will always show 0 passes. Please either hook this logic up in inspectGiverRow (populate skippedTetrathrinOfferIds and skip those offers) or remove the unused set and status to avoid misleading output.
| @Number(min = 0.10, max = 0.90, step = 0.05) | ||
| public double minimumHpPercent = 0.30; | ||
|
|
||
| public boolean stopWhenAllDailyTasksAreDone = true; |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Configuration fields maxMenuScanPasses and stopWhenAllDailyTasksAreDone are never honored by the module.
These two config options are defined but never influence DailyTaskAPI behavior: menuScanPass is never checked against maxMenuScanPasses, and finishAll() always sets state to DONE and stops the bot regardless of stopWhenAllDailyTasksAreDone. Please either wire these fields into the logic or remove them from the config to avoid misleading, unused settings.
| super(api); | ||
| } | ||
|
|
||
| void tick(String description) { |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Force-enabling PET via config and then disabling it on stop may conflict with user settings and other modules.
tick forces pet.enabled to true on every call, and stopCombat turns it off without regard for the previous value. This overrides the user’s global PET preference and can conflict with other modules using pet.enabled. Please preserve and restore the original setting or delegate PET control to DailyNpcPetLocator/the PET manager instead of mutating the shared config directly.
Suggested implementation:
final class DailyNpcCombat extends LootModule {
private String targetDescription;
private Boolean originalPetEnabled; void tick(String description) {
targetDescription = description;
// Capture the original PET enabled state once, then delegate control to the PET manager
if (originalPetEnabled == null) {
originalPetEnabled = pet.isEnabled();
}
if (!pet.isEnabled()) {
pet.setEnabled(true);
}
super.onTickModule();
} void stopCombat() {
attack.stopAttack();
attack.setTarget(null);
targetDescription = null;
// Restore PET state to whatever it was before this module started manipulating it
if (originalPetEnabled != null && pet.isEnabled() != originalPetEnabled) {
pet.setEnabled(originalPetEnabled);
}
originalPetEnabled = null;
}This change assumes:
- The
petfield is a PET manager/DailyNpcPetLocator that exposesisEnabled()andsetEnabled(boolean)and internally respects the globalpet.enabledconfig. tickis only called while this module is active; if other modules also toggle PET, you may need a more sophisticated ownership/priority mechanism in the PET manager to avoid conflicts.- If PET can be
nullin this class, wrap allpetaccesses in null checks.
If other parts of the file cached or manipulatedpet.enabledviaConfigAPI, those should be updated to use the PET manager as well to keep all PET control centralized.
Comment on lines
+77
to
+80
| require(DailyTaskPlanner.hasUridiumReward(List.of( | ||
| reward("currency_experience", 1000), reward("currency_uridium", 1500))), | ||
| "Uridium reward must be detected from the source reward type"); | ||
| require(!DailyTaskPlanner.hasUridiumReward(List.of(reward("resource_tetrathrin", 10))), |
Contributor
There was a problem hiding this comment.
suggestion (testing): Missing tests for progress(...) and timer-related requirement handling in the planner
The planner’s progress(...) and actionableIncludingCompleted(...) logic (zero-goal requirements, completed vs. non-completed, and exclusion of haste/timer types) is currently untested. Please add tests that cover quests with timer requirements, zero-goal requirements, and partially completed requirements, and assert the resulting progress values so we verify the daily completion detection and 99.99% threshold behavior used by DailyTaskAPI.
dm94
requested changes
Aug 2, 2026
dm94
left a comment
dm94
left a comment
Member
There was a problem hiding this comment.
- Comments and messages in English
- It doesn’t meet the minimum code quality standards
Have you checked that it works?
zrqq31
changed the title
Member
|
Each feature in a different folder, and as I’ve already mentioned, clean up the code first |
Author
|
Okay im handling this now.
Daniel Martín ***@***.***> şunları yazdı (11 Ağu 2026 23:25):
[https://avatars.githubusercontent.com/u/7419213?s=20&v=4]dm94 left a comment (Darkbot-Plugins/SharedPlugin#197)<#197 (comment)>
Each feature in a different folder, and as I’ve already mentioned, clean up the code first
—
Reply to this email directly, view it on GitHub<#197?email_source=notifications&email_token=CKLXIWZWIFJRGDRBKBSJ4UL5JN6LZA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRVHA2DINZVGQ3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5258447546>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/CKLXIW5G6OBODJ77VATUAC35JN6LZAVCNFSNUABGKJSXA33TNF2G64TZHMYTCMJXGU3TIMJYHE5US43TOVSTWNJQGM4TKMRXGY3THILWAI>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/CKLXIW3N22AVX475SEOQ2VT5JN6LZA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRVHA2DINZVGQ3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/CKLXIWYKFIV73TUNPXRJBIT5JN6LZA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMRVHA2DINZVGQ3KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Author
Member
dm94
requested changes
Sep 1, 2026
dm94
left a comment
dm94
left a comment
Member
There was a problem hiding this comment.
On the other hand:
- I don’t see the point of DailyNpcPetLocator
- The functionality of DailyRamCleaner should be in a separate pull request
dm94
reviewed
Sep 1, 2026
dm94
reviewed
Sep 1, 2026
zrqq31
changed the title
Author
|
Addressed the latest review in
Verification: the main sources compile with @dm94 Could you please re-review the latest commit? |
Member
|
/sourcery-ai review |
Member
I’ll ask again: have you tried this feature? Could you show a video or something similar of how it works? |
Author
|
Hi @dm94, I’m developing this feature with assistance from ChatGPT. I previously tested an earlier local revision, but I can’t test the current PR revision because DarkBot rejects the unsigned JAR. The contribution guide mentions that a test build can be provided for pull requests. Could you please provide a signed test build for the current commit, or let me know the supported testing procedure? Once I can load it, I’ll test the complete workflow manually, record a video demonstrating it, and fix any runtime issues before requesting final approval. |
Member
You need to run Darkbot from the IDE with signature checks disabled |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DailyTaskAPI, a standalone module that discovers supported daily quests from DarkBot quest data and completes NPC, player-combat (opt-in), map, coordinate, collection and ore objectives.loot.npc_infosmetadata to resolve NPC maps instead of maintaining a hard-coded NPC/map list.DailyTaskAPI; there is no separate locator feature.Scope
The RAM cleaner was removed from this pull request and will be submitted separately, as requested.
Verification
javac --release 11against DarkBotAPI 0.9.8.DailyTaskPlannerTest: 14/14 tests passed locally.plugin.jsonregisters onlyDailyTaskAPIfor this contribution.