Conversation
|
@holzmaster passt das grob in dein lootkonzept? |
|
Wär jetzt in son nem Alpha-stadium, das ist auch viel shitcode dabei, gerne ein code review oder direkt überarbeiten, wenn einer bock hat. Feedback zu der nicht technischen seite wär auch nicht schlecht |
holzmaster
left a comment
holzmaster
left a comment
There was a problem hiding this comment.
Ich hab nur mal grob drübergescrollt und das eigentliche Feature bzw. die business-logic noch nicht angeschaut
| ) | ||
| .addSubcommand( | ||
| new SlashCommandSubcommandBuilder() | ||
| .setName("ausrüsten") |
There was a problem hiding this comment.
Vielleicht passt hier "anlegen" oder "tragen" besser. Zumindest glaube ich, dass sich /gegenstand anlegen oder so verständlicher ist als /gegenstand ausrüsten
There was a problem hiding this comment.
Oder kollidiert das mit "benutzen"?
There was a problem hiding this comment.
anlegen finde ich tatsächlich passender.
Meinst du das benutzen und anlegen eigenlich das selbe tut? noch könnte man beides in ein command machen, aber potentiell gibt es items die man im chat und im kampf nutzen kann, daher würd ich das getrennt lassen
There was a problem hiding this comment.
Benutzen ist etwas, was in dem moment eine Aktion ausführt. Also z.b. ein Geschenk in den Chat droppt
| const equippedStuff = await getItemsByType(userId, itemType, ctx); | ||
| for (let i = 0; i <= equippedStuff.length - maxItems[itemType]; i++) { | ||
| const unequipitem = await lootService.getUserLootById( | ||
| userId, | ||
| equippedStuff[i].lootId, | ||
| ctx, | ||
| ); | ||
| unequippeditems.push(unequipitem?.displayName ?? String(equippedStuff[i].lootId)); | ||
| await ctx.deleteFrom("fightinventory").where("id", "=", equippedStuff[i].id).execute(); | ||
| } |
There was a problem hiding this comment.
Vielleicht kann man das hier einfacher machen mit sowas:
const equippedStuff = await getItemsByType(userId, itemType, ctx);
const excessItems = equippedStuff.slice(maxItems[itemType]);
for (const toRemove of excessItems) {
// ...
}There was a problem hiding this comment.
das ist viel schöner, thx
| @@ -0,0 +1,29 @@ | |||
| import { sql, type Kysely } from "kysely"; | |||
| import { createUpdatedAtTrigger } from "@/storage/migrations/10-loot-attributes.js"; | |||
There was a problem hiding this comment.
Die funktion kannste ruhig rüberkopieren
There was a problem hiding this comment.
oder in eine sql- migration- helper auslagern?
There was a problem hiding this comment.
Migrationen sollten jeweils komplett self-contained und "in der Zeit eingefroren" sein. Sonst wird irgendwann mal irgendwas minimales in dem Shared-Teil geändert und dann schlagen alte migrationen fehl, wenn man ein neues Dev-System aufsetzt
|
Ich hab mal die Konflikte gelöst und master reingemergt |
# Conflicts: # src/commands/fight.ts # src/service/fight.ts # src/service/loot.ts # src/storage/fightHistory.ts # src/storage/fightInventory.ts
2 participants
Moin, hab hier mal ein Kampfsystem gebastelt um items loszuwerden. (Bzw zum tauschen)
Noch lange nicht fertig, hier noch die TODOs:
Eventuelle noch ne karte zum reisen aber das kommt in V2.
Wer bock hat kann sich mein shitcode mal anschauen.
Ideen gerne, umsetzung noch viel besser