X Tutup
Skip to content

Feature/game manager refactor#135

Open
omarbitar97 wants to merge 5 commits intoPokemonUnity:masterfrom
omarbitar97:feature/game-manager-refactor
Open

Feature/game manager refactor#135
omarbitar97 wants to merge 5 commits intoPokemonUnity:masterfrom
omarbitar97:feature/game-manager-refactor

Conversation

@omarbitar97
Copy link

@omarbitar97 omarbitar97 commented Dec 8, 2024

Status

  • Ready
  • In development
  • Hold

Description

Testing

Concerns, notes, etc.

Related issues

Related PRs

- Enforced singleton pattern to prevent duplicate instances.
- Improved logging by encapsulating event handling logic.
- Enhanced database initialization with better error handling.
- Added validation for scene loader and localization file existence.
- Included meaningful comments for improved code readability.
- Enforced singleton pattern to prevent duplicate instances.
- Improved logging by encapsulating event handling logic.
- Enhanced database initialization with better error handling.
- Added validation for scene loader and localization file existence.
- Included meaningful comments for improved code readability.
@herbertmilhomme
Copy link
Member

Sorry, i didnt see this pull request until now, and i'm reviewing the changes. I saw that the title says it was for maintainability, but some of the commented out logic added to keeping things easier to maintain and well understood. Pending future changes with ToDos, as well as old testing logic for when unity stored files in different file paths and locations on hard disk, global logging agent for both handling logs through unity engine and through the framework for record keeping in a static text file, and some other function logics that were used as process of initializing the framework and game application.

I noticed you broke down some of the logic into their own method functions, which adds to things being a little clearner, but i believe that a merge of both cleaning and existing [commented] notation/annotations could have a balance in keeping things maintainable. Losing track of ToDos makes future changes harder to implement and track, and losing track of older edits makes it possible for previous pitfalls to reoccur as oversights. Maybe more text to explain older logic, can be added, but i felt there was a purpose and reason for why they were there (but commented). I did intend to uncomment, but had trouble determining how to address bringing the concepts and ideas to life. Thank you for actually adding implementation to some of the unresolved ideas.

I'll pull your changes on to my machine, and try to perform a merge that combines both old and new.

@herbertmilhomme
Copy link
Member

herbertmilhomme commented Dec 24, 2024

The game manager class is to be used by the Unity inspector for managing the Game (pokemon framework) class. Setting the fields as properties with private setters makes them incompatible with unity's monobehavior. I removed and reverted, while keeping the added notations and commentary. That way it remains informative while keeping functionalities intact. When the designer builds and integrates the framework into their game application, they will need some way to manage and interact with its configuration, and that's where the game object singleton comes into play.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup