Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimized the code for offsets.py #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Srishtiops
Copy link

This update includes key refactoring and optimizations for improved maintainability:

Constants for Base Offsets: Defined BASE_OFFSET and CHARACTER_OFFSET constants to avoid hardcoding and simplify future adjustments.

Refactored Character Offsets into Classes: Created a Character base class with common attributes (hp, mp, exp, etc.), with each character as a subclass to reduce duplication.

Added Missing Attributes: Added strength and magic attributes to all characters for consistency.

Organized Item Offsets: Grouped item offsets by type (healing, offensive, etc.) for easier reference and updating.

@mepley1
Copy link
Owner

mepley1 commented Nov 10, 2024

It's better than mine at least. I'll take it. Thanks

Sorry I didn't see this before October ended- I've given it a Hacktoberfest accepted label anyways. I'm in the middle of moving IRL so I don't have everything set up right now.

The only possible issue I can see, without having my development environment on-hand right now to test: Let's either separate the re-organized inventory items into a separate PR, or wait to merge. I'll have to fix the OptionMenu code in the GUI to read/display them properly before merging. The rest looks like it shouldn't require any other code changes.

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

Successfully merging this pull request may close these issues.

2 participants