LunarTides/Hearthstone.gd
1
0
Fork 0
Code Issues 1 Pull requests 2 Projects Releases Packages Wiki Activity Actions

Split card types into modules #7

Merged
LunarTides merged 2 commits from type-modules into main 2024-04-24 06:37:00 +00:00
Conversation 5 Commits 2 Files changed 87 +1771 -391
LunarTides commented 2024-04-02 09:05:13 +00:00 (Migrated from github.com)
Copy link

Split the different card types: Minion, Spell, Hero, Hero Power; into their own modules.
This also splits some additional stuff: Armor (The mesh and label), Attack (The mesh and label), and Health (The mesh and label).

TODO:

  • Split hero power.
  • Fix card-type specific meshes only appearing on The Coin.
  • Fix client-side checks.
  • Fix spell school label not being added to cards.
  • Fix summoning cards not working.
  • Fix health not showing up on heroes.
  • Update wiki.
Split the different card types: `Minion`, `Spell`, `Hero`, `Hero Power`; into their own modules. This also splits some additional stuff: `Armor (The mesh and label)`, `Attack (The mesh and label)`, and `Health (The mesh and label)`. TODO: - [x] Split hero power. - [x] Fix card-type specific meshes only appearing on The Coin. - [x] Fix client-side checks. - [x] Fix spell school label not being added to cards. - [x] Fix summoning cards not working. - [x] Fix health not showing up on heroes. - [x] Update wiki.
LunarTides commented 2024-04-04 19:36:21 +00:00 (Migrated from github.com)
Copy link

Some of these changes should get cherrypicked, will do tomorrow.

Some of these changes should get cherrypicked, will do tomorrow.
LunarTides (Migrated from github.com) reviewed 2024-04-19 08:23:52 +00:00
LunarTides (Migrated from github.com) left a comment
Copy link

Remember to also resolve the review comments from #4

This is far from perfect, and there are some critical problems and design flaws (also it doesn't even work) so I can't do an early merge to fix stuff later.

Remember to also resolve the review comments from #4 This is far from perfect, ~and there are some critical problems and design flaws (also it doesn't even work) so I can't do an early merge to fix stuff later.~
modules/armor/armor.gd
@ -0,0 +1,91 @@
extends Module
LunarTides (Migrated from github.com) commented 2024-04-19 08:12:12 +00:00
Copy link

Is registering hooks on behalf of another module a good idea?

Is registering hooks on behalf of another module a good idea?
modules/attack/attack.gd
@ -0,0 +1,80 @@
extends Module
LunarTides (Migrated from github.com) commented 2024-04-19 08:03:04 +00:00
Copy link

This should be removed

This should be removed
modules/attack/attack.gd
@ -0,0 +75,4 @@
attack_label.text = str(card.attack)
attack_label.visible = attack_visible
return true
LunarTides (Migrated from github.com) commented 2024-04-19 08:03:52 +00:00
Copy link

Something like this was done in the armor module. Maybe have a helper function for this?

Something like this was done in the armor module. Maybe have a helper function for this?
modules/location/layout/hero/hero.gd
@ -23,1 +31,4 @@
return true
func layout_hero(card: Card) -> Dictionary:
LunarTides (Migrated from github.com) commented 2024-04-19 08:04:37 +00:00
Copy link

Too many newlines.

Too many newlines.
modules/rarity/rarity.gd
LunarTides (Migrated from github.com) commented 2024-04-19 08:19:34 +00:00
Copy link

This should be done in main.

This should be done in main.
modules/type/hero_power/hero_power.gd
@ -0,0 +58,4 @@
if hero_power.refunded:
return false
player.has_used_hero_power_this_turn = true
LunarTides (Migrated from github.com) commented 2024-04-19 08:10:06 +00:00
Copy link

I don't think Player.has_used_hero_power_this_turn should be in core.

I don't think `Player.has_used_hero_power_this_turn` should be in core.
modules/type/hero_power/hero_power.gd
@ -0,0 +100,4 @@
Anticheat.feedback("Invalid hero power info.", sender_peer_id)
return false
var hero_power: Card = actor_player.hero.hero_power
LunarTides (Migrated from github.com) commented 2024-04-19 08:09:29 +00:00
Copy link

I don't think Player.hero or Card.hero_power should be in core, I'm not sure what the alternative solution is.

I don't think `Player.hero` or `Card.hero_power` should be in core, I'm not sure what the alternative solution is.
modules/type/minion/minion.gd
@ -0,0 +1,81 @@
extends Module
LunarTides (Migrated from github.com) commented 2024-04-19 08:14:46 +00:00
Copy link

I think this is gross since we are summoning the card after triggering the ability, which might be confusing to card creators if they don't know the reasoning behind it. This should be documented somewhere, but in the meantime, rephrase this comment with the reasoning above.

Also we should add Card.summon so we can do this instead:

card.summon(board_index, false)
I think this is gross since we are summoning the card after triggering the ability, which might be confusing to card creators if they don't know the reasoning behind it. This should be documented somewhere, but in the meantime, rephrase this comment with the reasoning above. Also we should add `Card.summon` so we can do this instead: ```gdscript card.summon(board_index, false) ```
modules/type/minion/tribe/tribe.gd
@ -0,0 +71,4 @@
return true
# TODO: Return Array[StringName] in 4.3
LunarTides (Migrated from github.com) commented 2024-04-19 08:17:18 +00:00
Copy link

Why can we only do this in 4.3? Is there a pr that can be linked to?

Why can we only do this in 4.3? Is there a pr that can be linked to?
scripts/blueprint_manager.gd
@ -29,4 +29,2 @@
Card._update_card(card, blueprint)
# /////////////// Suggest ID ///////////////
LunarTides (Migrated from github.com) commented 2024-04-19 08:18:14 +00:00
Copy link

We should somehow add this back.

We should somehow add this back.
LunarTides (Migrated from github.com) reviewed 2024-04-23 08:50:32 +00:00
modules/armor/armor.gd
@ -0,0 +1,91 @@
extends Module
LunarTides (Migrated from github.com) commented 2024-04-23 08:50:32 +00:00
Copy link

yesno

yesno
LunarTides (Migrated from github.com) reviewed 2024-04-23 08:51:02 +00:00
scripts/blueprint_manager.gd
@ -29,4 +29,2 @@
Card._update_card(card, blueprint)
# /////////////// Suggest ID ///////////////
LunarTides (Migrated from github.com) commented 2024-04-23 08:51:01 +00:00
Copy link

This should probably be done in a follow-up pr.

This should probably be done in a follow-up pr.
LunarTides commented 2024-04-24 06:31:11 +00:00 (Migrated from github.com)
Copy link

Merging prematurely again. Remember to resolve the review comments here and in #4 in the future.

Merging prematurely again. Remember to resolve the review comments here and in #4 in the future.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something isn't working

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
wontfix
This will not be worked on

No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
LunarTides/Hearthstone.gd!7
No description provided.