[SM64] Combined Export Panel & Behavior Script Exporting #284
Conversation
…t exporting support
…l selected objects. Edited custom path exports. Cleaned up code
…ting bhv with no bhv
There was a problem hiding this comment.
Export level SHOULD NOT be in the same panel as combined object exports, especially if the name of the panel is "SM64 Combined Object Exporter", the positioning of the level prop and the fact that this will cause confusion while exporting for both actor exports and level exports.
|
I'm unsure if I like stuff like "Export behavior" and "Export collision" to be on by default |
|
All behavior scripts should start with a BEGIN command, maybe don´t make the user add those manually and just add an enum for the object list? Also maybe adding a warning for when the behavior script doesn´t contain an END_LOOP, BREAK, DEACTIVATE, GOTO or RETURN could be a neat addition (only a warning because the user could have their own custom command which exits out of the behavior script). And if you are willing to step into feature creep territory, having toggles for common vanilla object flags would be very convenient. I don´t know what else could be done to help the user not mess up the behaviors. |
fast64_internal/sm64/sm64_objects.py
Outdated
| prop_split(col, self, "collision_object", "Collision Obj") | ||
| if self.export_gfx and not self.export_all_selected: | ||
| prop_split(col, self, "graphics_object", "Geo Layout Obj") |
There was a problem hiding this comment.
Is there a reason to abbreviate these?
Also fast64 uses "Geolayout" instead of "Geo Layout" everywhere else
| insert_line = 0 | ||
| alt_insert_line = 0 | ||
| match_line = 0 | ||
| for j, line in enumerate(file_lines): |
There was a problem hiding this comment.
Is there any reason you use j over i for loops? Typically you use j for a second index and i for the first index
There was a problem hiding this comment.
habits from electrical studies no real reason
fast64_internal/sm64/sm64_objects.py
Outdated
|
|
||
| def export_group_script_load(self, script_path, props): | ||
| if not script_path.exists(): | ||
| PluginError(f"could not find {script_path.stem}") |
There was a problem hiding this comment.
A lot of your plugin errors start with lower case, but some of yours and all others in fast64 use normal casing.
| return {"CANCELLED"} | ||
|
|
||
| props.context_obj = None | ||
| # you've done it!~ |
fast64_internal/sm64/sm64_objects.py
Outdated
| if props.export_all_selected: | ||
| return | ||
| else: | ||
| raise Exception(e) |
There was a problem hiding this comment.
Why not just do if not props.export_all_selected:
There was a problem hiding this comment.
Unresolving this, but also I personally do this in these kinds of exceptions as much as possible, in here it may seem less useful but having a full traceback is usually good, pylint warns for it
| if props.export_all_selected: | |
| return | |
| else: | |
| raise Exception(e) | |
| except Exception as exc: | |
| # pass on multiple export, throw on singular | |
| if not props.export_all_selected: | |
| raise Exception(exc) from exc |
|
The latest commit should address all the review concerns and also added extra control over object naming. |
|
The big issue with this is hackersm64, what are your plans to address enum model ids and level scripts being included in the actors folder (was reverted for the time being but is still planned afaik, fazana said grep could be an easy solution for this one) |
|
I have no plans to address a change not yet implemented in a specific repo. All features of this panel can be enabled/disabled independently, if a specific repo does not work with a feature, they can turn it off. If there is support for specific repos and versions of repos I may address this but I don't believe I have any measure currently to handle this. |
|
bump #284
|
There was a problem hiding this comment.
Went over all the code again, aside from updating to main these are the last things (I'm sorry for dragging this out but I swear I don´t think I missed anything this time, I re-read all the changes and see no issues and I tested all types of exports one more time, so after this I will accept and any leftover issues will be on me lol)
|
@Reonu im gonna merge in like 2 days max, if you wanna test anymore do it soon please |
|
Since my last comment I've tested exporting behaviors and collision, it all worked fine. My only complaint is that the behavior scripts exported by fast64 are indented differently than the vanilla scripts (no extra indentation after BEGIN_LOOP) |
|
I don´t see an issue with that honestly, fast64 doesn´t always stick to what vanilla does for identation |
|
It matters more for behavior script exports than geos because people actually still have good reasons to interface with the behavior script file directly. Geos are exported to their own files and are less necessary to adhere to matching formatting. |
|
@jesusyoshi54 do you wanna address this before merging? |
|
|
|
I was about to tell you to merge this and then I realized it was already merged Then I was going to tell you to fix wiseguy decals but then I realized you already did Thank you lila |
This PR aims to streamline exporting for SM64 all into one panel. This PR is made for C exclusively, and binary features should remain unchanged.
Along with all of the above improvements to exporting. I have also added in behavior script exporting.
Behavior scripts will be exported using the same properties as above, and they also have the option to inherit the following properties from the exported object:
Behavior scripts come with several preset behaviors and support adding, deleting and editing args of all the existing original sm64 behavior script cmds.