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

Add Trigger Editor #24

Merged
merged 18 commits into from
Apr 29, 2023

Conversation

derneuere
Copy link
Contributor

This pull request adds another property grid for editing the triggers. It also implements reading and writing section4Entry data.

@burninrubber0
Copy link
Owner

Similar situation to #22, lots of offsets and counts and such that should be hidden. A 3D editor would be best but after reading what was said in Discord that probably won't be feasible for the foreseeable future, so the collection editor works well enough here.

On a personal note, it's good to see an editor for triggers going public, so keep it coming :)

Copy link
Owner

@burninrubber0 burninrubber0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few issues came up in testing. The biggest one is that almost all triggers (GenericRegions, Killzones, RoamingLocations, and others) crashed BM upon pressing OK after having one entry added to their array. It looks like this is because certain objects (mainly base TriggerRegions and Vector3s) are not created. Given the way it's done, you might have to fix and test each trigger type individually.

When the editor got far enough to save the edited bundle, attempting decompression afterward crashed DGI's Noesis unpacker. I didn't test it with the game, but in the past this has always translated to in-game issues. It's possible this is an existing issue with BM's Bundle code but it's hard to say without further troubleshooting.

Aside from these bugs, the editor itself looks decent. The only thing I'd recommend is automatically recalculating muSize upon saving and not exposing the field to the user. Maybe I'll have more suggestions after fixes are implemented, but the way the editor works now seems alright.

@derneuere
Copy link
Contributor Author

The muSize seems to get recalculated correctly, but I think you have to close and open it again.
Fixed the issues, seemed to be that I didn't initialize the values.

I am unfamiliar with DGI's Noesis unpacker. Any steps I could follow to test that?

Copy link
Owner

@burninrubber0 burninrubber0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest commit fixed the majority of the issues, but there are still crashes when opening a new Killzone's RegionIds collection editor or opening either of StartingGrid's Vector3I collection editors. Otherwise, it seems stable.

In the editor, TriggerRegion's mBoxRegion needs to be editable. CgsIDs also appear read-only unless part of a collection (see the SignatureStunt and SpawnLocation editors).

It might also be worth using enums for the Blackspot ScoreType and GenericRegion Type like the TriggerRegion Type does, as well as setting the correct TriggerRegion Type on Blackspots and GenericRegions like is done with VFXBoxRegions.

I looked through the output file after making a few changes and everything looked correct, so after the above is addressed it should be good to go. I confirmed the Bundle issue mentioned earlier is a separate, known bug, #13.

@derneuere
Copy link
Contributor Author

I can't really reproduce the issue with the killzone region IDs.
StartingGrids should not get saved yet, as it's unclear where it's saved within the file.

Added enums and editors for Vector3I and CgsIDs and explained that you have to use the region.mid for trigger ids in killzone :)

@burninrubber0
Copy link
Owner

Clicking Add in the Killzone editor now results in this:
image

Based on the triggers in the Feb 22 2007 build, StartingGrids are written after SpawnLocations in the file, but before the TriggerRegion pointers. Also of some relevance, stunt elements for SignatureStunts are written after StartingGrids. Neither of these have entries in any retail version of the game, so it's up to you whether to support them or not.

Previously read-only fields are editable now, so that looks good. Enums look good too, thanks for adding them.

@derneuere
Copy link
Contributor Author

derneuere commented Apr 27, 2023

Thanks for the info! Added writing for the starting grids and the signature stunt elements.
Adding should work now, had to refactor all arrays to lists :)

Edit: It saves the file at the wrong offsets atm. Will look into it

@derneuere
Copy link
Contributor Author

Alright, found the bug, should work now :)

@burninrubber0
Copy link
Owner

Nice, that's a good improvement, I think we're just about there for real now.

The Killzone region IDs issue mentioned earlier is still present. Here's a video of it so you can reproduce:
https://user-images.githubusercontent.com/17496749/234901161-a63d57d7-d34b-4b78-9c7b-57b7c5fb902d.mp4

When adding a new vector to either collection in a starting grid, I received this error:
image

Also, SignatureStunt's mId is not editable.

- Add allowing edting mId
- Switch to List for RegionIDs to hopefully resolve bug for burninrubber
- Add default constructor for Vector3I
@derneuere
Copy link
Contributor Author

Could not recreate the bug for RegionIDs even with the video. Weird. I switched it to a list, which should probably work for you too, as other lists seem to work.

Added a default constructor for Vector3I and made mId editable :)

@burninrubber0
Copy link
Owner

Yep, that fixed all the issues remaining. I'll take one last look and see if there's anything left to do before merging.

This was already done with VFX box regions, just applying it to the rest.
Spawn location type, generic region stunt camera type
Not a valid value, shouldn't be selectable in the editor
@burninrubber0
Copy link
Owner

Merging. Normally this would warrant a new release but I'll try to address #13 first.

@burninrubber0 burninrubber0 merged commit 10afe29 into burninrubber0:master Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants