Proper module support #4
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
LunarTides/Hearthstone.gd!4
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "modules"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #3
I was a little bit premature to close #3 by
cb790d7.Turned most Enums into
Array[StringName]to be able to add more at run-time.Known regressions:
The Coinstarts in the left of the player's hand.Todo:
I did a very rough pass of the code. Make sure everything is stable before merging.
Also rename the commits before merging.(Fixed)@ -1,19 +1,22 @@[gd_scene load_steps=4 format=3 uid="uid://c8kcj58hy41o0"]The taunt is just for testing. Remove before merging.
@ -10,3 +10,3 @@card.add_ability(Card.Ability.CAST, cast)card.add_ability(&"Cast", cast)Using
StringNameinstead of an enum makes it a lot easier to mistype. Figure out if there is some way to prevent that.Maybe instead of passing a handler, we could pass
self:And then in
modules.gd:@ -90,6 +90,94 @@ enum {The rarities will probably get their own module/modules. I just have them here for now, but in the future we would want to split them.
This applies to most stuff here. Splitting them out of core would be difficult though...
@ -37,1 +37,4 @@blueprints.sort_custom(func(a: Blueprint, b: Blueprint) -> bool:if not a.id or not b.id:return trueI'm not sure if this is necessary but it was giving me a lot of errors (though adding this didn't fix it).
Try to remove and see if it works.
@ -264,2 +266,4 @@Multiplayer.create_blueprint_from_id.rpc(2, player2.id, &"Hand", player2.hand.size())I wonder if it would be possible to put the code in
Packet.sendhere.We need to make sure that the
StringNameinfo stuff is validated everywhere.This is unrelated, I just spotted it.
Just in case any modules want to extend these for whatever reason.
In fact, maybe every module should inherit some
Moduleclass that does this in_readyautomatically. Then it would be less work and code for the individual modules.I think it would be better for users to specify themselves. It would also be different from how cards do it, since cards do something similar with abilities.
I did a pass of the two newest commits. It looks good, but there are some concerns. This will likely be the final review, since i need to merge this soon.
@ -17,3 +19,3 @@collectible = trueid = 3tribes = Array[int]([0])tribes = Array[StringName]([&"None"])Is this good? This would be a lot better in Godot 4.3 because of typed dictionaries.
@ -0,0 +1,28 @@extends NodeThis doesn't do anything yet, but it might in the future. Maybe it would be a good idea to not do this until needed?
@ -0,0 +2,4 @@#region Internal Functionsfunc _ready() -> void:Using modules directly is VERY BAD. The only exception is when using a base module inside of a child module. E.g. Using
KeywordModulein theTauntmodule, like we are doing here.This is very ugly. Figure out if there is a better way of doing this.
@ -0,0 +1,84 @@extends NodeFor some reason, rarities show for player 2 even if the rarity is
Free?We register all rarities in the base rarity module since individual rarities aren't unique enough to warrent their own seperate modules.
@ -0,0 +46,4 @@# Rarity Colorif not card.modules.get("rarities"):assert(false, "'%s' (%d) doesn't have a rarity." % [card.name, card.id])Is it a good idea to warn about not using a module? I guess using a disabled module isn't really a problem, so it should be fine.
@ -26,7 +26,17 @@ Packet="*res://scripts/multiplayer/packet.gd"Anticheat="*res://scripts/multiplayer/anticheat.gd"Don't expose
RarityModulesince it doesn't have any children? Maybe?@ -28,2 +28,3 @@Modules="*res://scripts/modules.gd"TauntModule="res://modules/taunt/taunt.gd"KeywordModule="*res://modules/keyword/keyword.gd"KeywordTauntModule="*res://modules/keyword/taunt/taunt.gd"KeywordModuleand all base modules need to be global variables to be used by child modules. There should be something to discourage referencing modules directly somewhere for other users, but it's fine for now.@ -9,0 +37,4 @@CARD_SUMMON,CARD_TWEEN_START,CARD_UPDATE,DAMAGE,Remove verbose printing since it was spamming the console with ~10,000 lines of garbage because of the "Update Card" request.
@ -0,0 +1,84 @@extends NodeAlso, the rarity mesh needs to be split from the core card model.
Should keywords be arrays or dictionaries?
@ -0,0 +1,84 @@extends NodeThis turned out to be a much bigger problem than I thought...
@ -26,7 +26,17 @@ Packet="*res://scripts/multiplayer/packet.gd"Anticheat="*res://scripts/multiplayer/anticheat.gd"Superseded by https://github.com/LunarTides/Hearthstone.gd/pull/4#discussion_r1525871216
@ -264,2 +266,4 @@Multiplayer.create_blueprint_from_id.rpc(2, player2.id, &"Hand", player2.hand.size())No longer applies as of
517a342291Merging prematurely to just get this merged.
@ -10,3 +10,3 @@card.add_ability(Card.Ability.CAST, cast)card.add_ability(&"Cast", cast)Making them capitalized makes it worse. Maybe think about making it lowercase / uppercase.