Revert parts of ascii_to_model.py to better support RCRA

[PythonBlue]

Fixed some breaking issues for model reimporting specific to RCRA. Sadly, a good chunk of the fixes involved reverting code to 0.3.1, the last release that worked for the game insofar as my tests. Also had to omit the mesh clearing method because otherwise the game would crash for me after enough elapsed time. Revisions for the sake of compatibility with other games are welcome.

[Tkachov]

Hello!

Thanks for your interest in fixing the script! I'd like to request some revisions to your changes before this can be pulled.

First off, as you mention yourself, this script is supposed to work with other games too, so I'd like it not affect these. It could be done somewhat like this:

if self.mode == dat1lib.VERSION_RCRA:
	# RCRA impl
else:
	# MSM impl

So, instead of reverting update_lookgroups(), you should rather have a branch like that. Also, I don't really think that old implementation was better, because it only updates look #0 and forces LOD #0. New implementation uses lookgroups_overrides read from 'materials.txt', allowing user more control over what gets written to the lookgroups section.

---

I find the return in update_meshes() weird. First, it's inside of a for loop, so it probably should've been continue — otherwise, only the first mesh gets updated. But second, it means that self.refresh_section(SECTION_MESHES) never happens, so basically the entire method does nothing. If that was your intention, you should've put early return in the very beginning of the method. But that's also strange decision, because that means user wouldn't be able to update meshes data (number of vertexes, indexes, etc) — so, can't add or remove vertexes or polygons. That does allow for some model adjustments, like resculpting, but limits user dramatically. I don't think that's what you wanted.

---

The change from if has_rcra_skin: to if self.mode == dat1lib.VERSION_RCRA: in inject_vertexes() is also strange. This means script would attempt to edit self.rcra_weights_section regardless of whether this model has such section or not. Maybe relying on the flag also isn't the most foolproof solution, but assuming that all models have such section isn't correct. Plus, while I do call this section "RCRA skin", I think it's actually present in MSM2 too. So, if someone in the future would like to adapt the script to other games, this part would work incorrectly, because it'd be checking for RCRA mode and not the flag.

I guess what you could do is overwrite the flag value after it's original assignment somewhat like that:

if self.mode == dat1lib.VERSION_RCRA:
	has_rcra_skin = (self.rcra_weights_section is not None)

---

When I tried to run the script with your changes, I've got the following error:

TabError: inconsistent use of tabs and spaces in indentation

Maybe you've got some weird version of Python that doesn't care about tabs and spaces being mixed up, but I don't. Either that, or you didn't even test the change. Please test your changes before submitting a PR.

---

And finally, I've tried the script on a few models locally, by simply injecting .ascii back, and they still appear broken in game. I actually see no difference, even though there are changes to the binary.

Can you provide information on how you tested your change? What models did you test, what were the changes, etc. I'd also like to see the before/after screenshots, showing that what was broken in the old version of the script now isn't.

If you did modify .ascii and/or .materials.txt before injecting them back, please provide them too, so I can verify it on my side.

[PythonBlue]

Oh, didn’t expect you to respond so quickly, thank you for that.

Ah, good to know some of the models you tested with are still broken…maybe for the sake of making sure we have the same code and tests, which Rift Apart models did you test? As my edit is now, I verified it can reimport ascii, both intentionally modified and otherwise, based on the Goons-4-Less models without any problems, sans any pre-importing modifications that cause rigging weirdness, whereas with the original code, not only would the vertices be all over the place, I’d be lucky if the game didn’t crash after only a few seconds of the model loaded, hence, why I had to skip the mesh clearing method you used in the code in particular.

Regardless, if we do have the same code and test files, I’ll address the rest of your feedback then to avoid any further premature actions, because if it really is still broken, then it may be worth investigating further before I commit (no pun intended) to making a PR…

[PythonBlue]

Take it back: I’ll respond to your second-to-last comment. I thought I had fixed the tabbing issue before I committed: apologies for that. Will verify which line it is and then try to recommit.

[Tkachov]

I've tested
characters/npc/npc_zurkon_jr/npc_zurkon_jr.model (94A4B69B67D5CC42)
and
characters/npc/npc_civ_robot_01/npc_civ_robot_01.model (8D98795E786B0206)

they are conveniently in the game's gallery, so it's quick to test

[PythonBlue]

I've tested characters/npc/npc_zurkon_jr/npc_zurkon_jr.model (94A4B69B67D5CC42) and characters/npc/npc_civ_robot_01/npc_civ_robot_01.model (8D98795E786B0206)

they are conveniently in the game's gallery, so it's quick to test

Understood. I'll check those next, then.

[PythonBlue]

I've tested characters/npc/npc_zurkon_jr/npc_zurkon_jr.model (94A4B69B67D5CC42) and characters/npc/npc_civ_robot_01/npc_civ_robot_01.model (8D98795E786B0206)
they are conveniently in the game's gallery, so it's quick to test

Understood. I'll check those next, then.

Alright, looked into Zircon Jr. and the Civilian just now. Strangely enough, the only problem I see is the lack of texture color for Zircon Jr., and even there, that might be the typical rendering issues on this Mac...will confirm that much on another machine later, but the geometry, my biggest concern, seems fine to me for both of them....

EDIT: Figured out why the inconsistency. Sending a third commit soon.

[PythonBlue]

Now that the code I verified is functionally identical to what I was running locally, I'm ready to address your other points:

• Fair point about branching. Again, you're free to revise that aspect to my commit as you see fit.

• while you are right about returning in a loop, in retrospect, the good news is that concern is negated with the newest commit

• insofar as the rcra_skin variable, ideally I wouldn't need to mess with that, myself, yet I noted that with the Goons-4-Less models, their eyeballs in particular were in incorrect positions unless I changed that section.

[Tkachov]

I've checked my .stage files with older Overstrike versions and then models appear fine (made with either latest version of the script or your change). It seems the way it's currently being installed somehow affects models.

[PythonBlue]

I've checked my .stage files with older Overstrike versions and then models appear fine (made with either latest version of the script or your change). It seems the way it's currently being installed somehow affects models.

Sadly, yeah, I noticed the same problem insofar as what version you install with, though that pretty much goes for any assets edited using this suite, as well as some tools by others…needless to say, I’ve been using Overstrike 1.5.1 and its corresponding version of ModdingTool for the sake of compatibility with the various tools out there.

[Tkachov]

I've updated Modding Tool (1.4.5), it now correctly extracts RCRA assets in STG containers (which were added for MSM2 last year). This means correctly extracted assets are correctly installed with latest Overstrike (1.7.2).

Since 'main' branch of this repo wasn't updated to support STG, I've made a new branch, 'rcra', that has STG support. There are model<->ascii scripts in that branch too, and I've tested that these work with 'npc_civ_robot_01.model' just fine.

Feel free to test these scripts and updated tools, and tweak the scripts in 'rcra' branch any way you'd like as these don't have to support other games anymore.

[PythonBlue]

Thanks for the updates!