db0 / godot-card-game-framework

A framework which comes with prepared scenes and classes to kickstart your card game, as well as a powerful scripting engine to use to provide full rules enforcement.
http://dbzer0.com/projects/godot-card-game-framework/
GNU Affero General Public License v3.0
952 stars 100 forks source link

change CardContainer and change card to group #42

Closed vmjcv closed 3 years ago

vmjcv commented 3 years ago

I made the following changes

  1. use group to control button not get_children(), So that others don’t rely on the same structure
  2. use onready var manipulation_buttons = $Control/ManipulationButtons, Getting the node in advance can not get_node every time it is used, and can make the code shorter
  3. delete _on_button_mouse_entered,This can cause problems when the mouse moves
  4. Apply animation to buttons instead of ManipulationButtons
  5. use custom seed. This allows the same random number to lead to the same game
  6. separate _ready()
  7. modify get_all_cards and shuffle_cards
db0 commented 3 years ago

Thanks for participating. Could you please merge the latest changes from my main, put your changes into a branch, and then open a pull request via that branch? This allows me to review the changes easier.

Some issues I see:

In the future, I would appreciate if each component you change is its own branch. Then I can review them individually instead of having to review hundreds of line of multiple changes in functionality.

For example, this pull request could have been 4 distinct branches

Again, thanks for the effort. Once the GUT tests are running I can review again and see if any regression bugs were introduced.

db0 commented 3 years ago

BTW,I just wrote a contribution guide as well. Feel free to join me on discord if you need any help: https://discord.gg/AjZMFY7jD4

vmjcv commented 3 years ago

Thank you very much for your reply, I will try to cut it into different functional branches to complete the function (the first time I did a merge submission, I was a bit unskilled)

vmjcv commented 3 years ago

I will close this and create a few new merge codes