-
Notifications
You must be signed in to change notification settings - Fork 45
Combine MiscTablesModule and ExpandedTablesModule #326
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,57 +2,123 @@ | |||||||||||||||||
|
|
||||||||||||||||||
| from coilsnake.model.eb.table import eb_table_from_offset | ||||||||||||||||||
| from coilsnake.modules.eb.EbModule import EbModule | ||||||||||||||||||
| from coilsnake.util.common.yml import convert_values_to_hex_repr, replace_field_in_yml, yml_load, yml_dump | ||||||||||||||||||
| from coilsnake.util.eb.pointer import from_snes_address | ||||||||||||||||||
| from coilsnake.util.common.yml import (convert_values_to_hex_repr, | ||||||||||||||||||
| replace_field_in_yml, yml_dump, | ||||||||||||||||||
| yml_load) | ||||||||||||||||||
| from coilsnake.util.eb.pointer import (AsmPointerReference, XlPointerReference, | ||||||||||||||||||
| from_snes_address, to_snes_address) | ||||||||||||||||||
|
|
||||||||||||||||||
| log = logging.getLogger(__name__) | ||||||||||||||||||
|
|
||||||||||||||||||
| # this decorator is used to calculate the free ranges instead of hardcoding them. | ||||||||||||||||||
| # in 3.9 - 3.11, it's possible to combine @property and @classmethod, so we COULD just do that... | ||||||||||||||||||
| # however, for the sake for maintainability, a >3.11-style solution is here. | ||||||||||||||||||
| class classproperty: | ||||||||||||||||||
| def __init__(self, func): | ||||||||||||||||||
| self.fget = func | ||||||||||||||||||
| def __get__(self, instance, owner): | ||||||||||||||||||
| return self.fget(owner) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class MiscTablesModule(EbModule): | ||||||||||||||||||
| NAME = "Miscellaneous Tables" | ||||||||||||||||||
| TABLE_OFFSETS = [ | ||||||||||||||||||
| 0xC3FD8D, # Attract mode text | ||||||||||||||||||
| 0xD5F645, # Timed Item Delivery | ||||||||||||||||||
| 0xE12F8A, # Photographer | ||||||||||||||||||
| 0xD5EA77, # Condiment Table | ||||||||||||||||||
| 0xD5EBAB, # Scripted Teleport Destination Table | ||||||||||||||||||
| 0xD5F2FB, # Hotspots Table | ||||||||||||||||||
| 0xC3F2B5, # Playable Character Graphics Control Table | ||||||||||||||||||
| 0xD58A50, # PSI Abilities | ||||||||||||||||||
| 0xD57B68, # Battle Actions Table | ||||||||||||||||||
| 0xD5EA5B, # Statistic Growth Variables | ||||||||||||||||||
| 0xD58F49, # Level-up EXP Table | ||||||||||||||||||
| 0xD5F5F5, # Initial Stats Table | ||||||||||||||||||
| 0xD57880, # PSI Teleport Destination Table | ||||||||||||||||||
| 0xD57AAE, # Phone Contacts Table | ||||||||||||||||||
| 0xD576B2, # Store Inventory Table | ||||||||||||||||||
| 0xD5F4BB, # Timed Item Transformations | ||||||||||||||||||
| 0xD5F4CF, # Don't Care | ||||||||||||||||||
| 0xD55000, # Item Data | ||||||||||||||||||
| 0xC23109, # Consolation Item | ||||||||||||||||||
| 0xC3E250, # Windows | ||||||||||||||||||
| ] | ||||||||||||||||||
| NAME = "Misc Tables" | ||||||||||||||||||
| # Key: offset | ||||||||||||||||||
| # Value: a list of PointerReference-like objects to apply patches to expand the table, | ||||||||||||||||||
| # or a falsy value. A falsy value will be considered an unexpandable table. | ||||||||||||||||||
| TABLE_OFFSETS = { | ||||||||||||||||||
| 0xD58D7A: [ # PSI NAMES | ||||||||||||||||||
| AsmPointerReference(0x1c423) | ||||||||||||||||||
| ], | ||||||||||||||||||
| 0xCF8985: [ # NPC Configuration Table | ||||||||||||||||||
| AsmPointerReference(0x023E5), # in func C0222B/ Load NPCs | ||||||||||||||||||
| XlPointerReference(0x0C32E), # in func C0C30C | ||||||||||||||||||
| AsmPointerReference(0x131CA), # in func C13187/Talk to | ||||||||||||||||||
| AsmPointerReference(0x1327F), # in func C1323B/Check NPC | ||||||||||||||||||
| AsmPointerReference(0x1332F), # in func C1323B/Check NPC | ||||||||||||||||||
| XlPointerReference(0x1AD77), # in func C1AD42 | ||||||||||||||||||
| AsmPointerReference(0x1B20B), # in func C1AF74/Use overworld item | ||||||||||||||||||
| AsmPointerReference(0x464C1), # in func C464B5/Create prepared NPC | ||||||||||||||||||
| AsmPointerReference(0x46831), # in func C4681A | ||||||||||||||||||
| AsmPointerReference(0x46930), # in func C46914 | ||||||||||||||||||
| ], | ||||||||||||||||||
| 0xC3FD8D: False, # Attract mode text | ||||||||||||||||||
| 0xD5F645: False, # Timed Item Delivery | ||||||||||||||||||
| 0xE12F8A: False, # Photographer | ||||||||||||||||||
| 0xD5EA77: False, # Condiment Table | ||||||||||||||||||
| 0xD5EBAB: False, # Scripted Teleport Destination Table | ||||||||||||||||||
| 0xD5F2FB: False, # Hotspots Table | ||||||||||||||||||
| 0xC3F2B5: False, # Playable Character Graphics Control Table | ||||||||||||||||||
| 0xD58A50: False, # PSI Abilities | ||||||||||||||||||
| 0xD57B68: False, # Battle Actions Table | ||||||||||||||||||
| 0xD5EA5B: False, # Statistic Growth Variables | ||||||||||||||||||
| 0xD58F49: False, # Level-up EXP Table | ||||||||||||||||||
| 0xD5F5F5: False, # Initial Stats Table | ||||||||||||||||||
| 0xD57880: False, # PSI Teleport Destination Table | ||||||||||||||||||
| 0xD57AAE: False, # Phone Contacts Table | ||||||||||||||||||
| 0xD576B2: False, # Store Inventory Table | ||||||||||||||||||
| 0xD5F4BB: False, # Timed Item Transformations | ||||||||||||||||||
| 0xD5F4CF: False, # Don't Care | ||||||||||||||||||
| 0xD55000: False, # Item Data | ||||||||||||||||||
| 0xC23109: False, # Consolation Item | ||||||||||||||||||
| 0xC3E250: False, # Windows | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| # we're going to calculate FREE_RANGES at runtime instead of hardcoding it. | ||||||||||||||||||
| # makes the code easier to maintain. | ||||||||||||||||||
| @classproperty # for an explanation of this decorator, see above | ||||||||||||||||||
| def FREE_RANGES(cls): | ||||||||||||||||||
| ranges = [] | ||||||||||||||||||
| for offset, patches in cls.TABLE_OFFSETS.items(): | ||||||||||||||||||
| if not patches: continue # non-expandable, doesn't free up space | ||||||||||||||||||
| table = eb_table_from_offset(offset) | ||||||||||||||||||
| ranges.append((from_snes_address(offset), from_snes_address(offset+table.size-1))) | ||||||||||||||||||
|
Comment on lines
+72
to
+74
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Less code is needed here if you leave out
Suggested change
|
||||||||||||||||||
| return ranges | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| def __init__(self): | ||||||||||||||||||
| super(MiscTablesModule, self).__init__() | ||||||||||||||||||
| self.tables = [(from_snes_address(x), eb_table_from_offset(x)) for x in self.TABLE_OFFSETS] | ||||||||||||||||||
| self.tables = dict() | ||||||||||||||||||
| for table_offset in MiscTablesModule.TABLE_OFFSETS: | ||||||||||||||||||
| self.tables[table_offset] = eb_table_from_offset(table_offset) | ||||||||||||||||||
|
|
||||||||||||||||||
| def read_from_rom(self, rom): | ||||||||||||||||||
| for offset, table in self.tables: | ||||||||||||||||||
| table.from_block(rom, offset) | ||||||||||||||||||
| for offset, table in self.tables.items(): | ||||||||||||||||||
| table.from_block(rom, from_snes_address(offset)) | ||||||||||||||||||
|
|
||||||||||||||||||
| def write_to_rom(self, rom): | ||||||||||||||||||
| for offset, table in self.tables: | ||||||||||||||||||
| table.to_block(rom, offset) | ||||||||||||||||||
| for offset, table in self.tables.items(): | ||||||||||||||||||
| # unexpanded tables should remain in place | ||||||||||||||||||
| if self.TABLE_OFFSETS[offset]: | ||||||||||||||||||
| new_table_offset = rom.allocate(size=table.size) | ||||||||||||||||||
| else: | ||||||||||||||||||
| new_table_offset = from_snes_address(offset) | ||||||||||||||||||
|
|
||||||||||||||||||
| table.to_block(rom, new_table_offset) | ||||||||||||||||||
| log.info("Writing table '{}' @ ".format(table.name) + hex(to_snes_address(new_table_offset))) | ||||||||||||||||||
| # Write pointers for expansion, only if there is a list of pointers. Otherwise consider it unexpanded | ||||||||||||||||||
| if self.TABLE_OFFSETS[offset]: | ||||||||||||||||||
| for pointer in self.TABLE_OFFSETS[offset]: | ||||||||||||||||||
| if pointer.validate_structure(rom): | ||||||||||||||||||
| pointer.write(rom, to_snes_address(new_table_offset)) | ||||||||||||||||||
| else: | ||||||||||||||||||
| log.warn("Table relocation at %#x failed structure check - skipping...", pointer.offset) | ||||||||||||||||||
|
|
||||||||||||||||||
| def read_from_project(self, resource_open): | ||||||||||||||||||
| for _, table in self.tables: | ||||||||||||||||||
| for offset, table in self.tables.items(): | ||||||||||||||||||
| with resource_open(table.name.lower(), "yml", True) as f: | ||||||||||||||||||
| log.debug("Reading {}.yml".format(table.name.lower())) | ||||||||||||||||||
| table.from_yml_file(f) | ||||||||||||||||||
| # expanded table - recreate it with our new number of rows | ||||||||||||||||||
| if self.TABLE_OFFSETS[offset]: | ||||||||||||||||||
| log.debug("Reading {}.yml as an expanded table".format(table.name.lower())) | ||||||||||||||||||
| yml_rep = yml_load(f) | ||||||||||||||||||
| num_rows = len(yml_rep) | ||||||||||||||||||
| table.recreate(num_rows=num_rows) | ||||||||||||||||||
| table.from_yml_rep(yml_rep) | ||||||||||||||||||
| # unexpanded table - standard process | ||||||||||||||||||
| else: | ||||||||||||||||||
| table.from_yml_file(f) | ||||||||||||||||||
|
|
||||||||||||||||||
|
Comment on lines
+109
to
119
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick but code comments should apply to the state of the system at the point the comment “executes.” Try moving the comments into the bodies of the if self.TABLE_OFFSETS[offset]:
# expanded table - recreate it with our new number of rows
log.debug("Reading {}.yml as an expanded table".format(table.name.lower()))
# ...I think it reads clearer that way |
||||||||||||||||||
| def write_to_project(self, resource_open): | ||||||||||||||||||
| for _, table in self.tables: | ||||||||||||||||||
| for table in self.tables.values(): | ||||||||||||||||||
| with resource_open(table.name.lower(), "yml", True) as f: | ||||||||||||||||||
| table.to_yml_file(f) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -61,20 +127,20 @@ def upgrade_project(self, old_version, new_version, rom, resource_open_r, resour | |||||||||||||||||
| return | ||||||||||||||||||
| elif old_version == 3: | ||||||||||||||||||
| replace_field_in_yml(resource_name="item_configuration_table", | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Effect", | ||||||||||||||||||
| new_key="Action") | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Effect", | ||||||||||||||||||
| new_key="Action") | ||||||||||||||||||
| replace_field_in_yml(resource_name="psi_ability_table", | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Effect", | ||||||||||||||||||
| new_key="Action") | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Effect", | ||||||||||||||||||
| new_key="Action") | ||||||||||||||||||
| replace_field_in_yml(resource_name="psi_ability_table", | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="PSI Name", | ||||||||||||||||||
| value_map={0: None, | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="PSI Name", | ||||||||||||||||||
| value_map={0: None, | ||||||||||||||||||
| 1: 0, | ||||||||||||||||||
| 2: 1, | ||||||||||||||||||
| 3: 2, | ||||||||||||||||||
|
|
@@ -98,22 +164,22 @@ def upgrade_project(self, old_version, new_version, rom, resource_open_r, resour | |||||||||||||||||
| resource_delete("sound_stone_palette") | ||||||||||||||||||
|
|
||||||||||||||||||
| self.upgrade_project(old_version=old_version + 1, | ||||||||||||||||||
| new_version=new_version, | ||||||||||||||||||
| rom=rom, | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| resource_delete=resource_delete) | ||||||||||||||||||
| new_version=new_version, | ||||||||||||||||||
| rom=rom, | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| resource_delete=resource_delete) | ||||||||||||||||||
| elif old_version == 2: | ||||||||||||||||||
| replace_field_in_yml(resource_name="timed_delivery_table", | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Suitable Area Text Pointer", | ||||||||||||||||||
| new_key="Delivery Success Text Pointer") | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Suitable Area Text Pointer", | ||||||||||||||||||
| new_key="Delivery Success Text Pointer") | ||||||||||||||||||
| replace_field_in_yml(resource_name="timed_delivery_table", | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Unsuitable Area Text Pointer", | ||||||||||||||||||
| new_key="Delivery Failure Text Pointer") | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Unsuitable Area Text Pointer", | ||||||||||||||||||
| new_key="Delivery Failure Text Pointer") | ||||||||||||||||||
|
|
||||||||||||||||||
| with resource_open_r("timed_delivery_table", "yml", True) as f: | ||||||||||||||||||
| out = yml_load(f) | ||||||||||||||||||
|
|
@@ -125,32 +191,32 @@ def upgrade_project(self, old_version, new_version, rom, resource_open_r, resour | |||||||||||||||||
| f.write(yml_str_rep) | ||||||||||||||||||
|
|
||||||||||||||||||
| self.upgrade_project(old_version=old_version + 1, | ||||||||||||||||||
| new_version=new_version, | ||||||||||||||||||
| rom=rom, | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| resource_delete=resource_delete) | ||||||||||||||||||
| new_version=new_version, | ||||||||||||||||||
| rom=rom, | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| resource_delete=resource_delete) | ||||||||||||||||||
| elif old_version == 1: | ||||||||||||||||||
| replace_field_in_yml(resource_name="psi_ability_table", | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Target", | ||||||||||||||||||
| new_key="Usability Outside of Battle", | ||||||||||||||||||
| value_map={"Nobody": "Other", | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Target", | ||||||||||||||||||
| new_key="Usability Outside of Battle", | ||||||||||||||||||
| value_map={"Nobody": "Other", | ||||||||||||||||||
| "Enemies": "Unusable", | ||||||||||||||||||
| "Allies": "Usable"}) | ||||||||||||||||||
| replace_field_in_yml(resource_name="battle_action_table", | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Direction", | ||||||||||||||||||
| value_map={"Party": "Enemy", | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| key="Direction", | ||||||||||||||||||
| value_map={"Party": "Enemy", | ||||||||||||||||||
| "Enemy": "Party"}) | ||||||||||||||||||
|
|
||||||||||||||||||
| self.upgrade_project(old_version=old_version + 1, | ||||||||||||||||||
| new_version=new_version, | ||||||||||||||||||
| rom=rom, | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| resource_delete=resource_delete) | ||||||||||||||||||
| new_version=new_version, | ||||||||||||||||||
| rom=rom, | ||||||||||||||||||
| resource_open_r=resource_open_r, | ||||||||||||||||||
| resource_open_w=resource_open_w, | ||||||||||||||||||
| resource_delete=resource_delete) | ||||||||||||||||||
| else: | ||||||||||||||||||
| self.upgrade_project(old_version + 1, new_version, rom, resource_open_r, resource_open_w, resource_delete) | ||||||||||||||||||
| self.upgrade_project(old_version + 1, new_version, rom, resource_open_r, resource_open_w, resource_delete) | ||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm misunderstanding something, you should be able to combine
@staticmethodand@property, instead of using@classmethod.This is the only spot in CoilSnake where FREE_RANGES is read from:
CoilSnake/coilsnake/ui/common.py
Line 149 in e12aeae
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been almost a year so I don't recall the details, but I remember this was giving me a headache. That means I probably landed on this for a reason, so a change would need to be tested