Skip to content

Lab Weapon Viewer#6704

Merged
wookieejedi merged 7 commits into
scp-fs2open:masterfrom
MjnMixael:lab_weapon_viewer
May 26, 2025
Merged

Lab Weapon Viewer#6704
wookieejedi merged 7 commits into
scp-fs2open:masterfrom
MjnMixael:lab_weapon_viewer

Conversation

@MjnMixael

@MjnMixael MjnMixael commented May 1, 2025

Copy link
Copy Markdown
Contributor

Activates the F3 Lab's weapon viewer mode. Outside of the lab-specific code there's a few changes worth mentioning here as I expect them to come up during review:

OBJ_RAW_POF is a new object type specifically designed for rendering a model file as an Object. The reasoning is because the only other way to do it would be to hack in a placeholder Ship_info, Weapon_info, or Asteroid_info class with dummy values just so the associated model rendering method would be able to pass all the usual checks for those object types. With this we do a straight render call of the POF without any of the other junk and no hacks to make it work. These objects have no collisions and no ai. There is potential to expand this to other useful applications in the future.

Weapon life is reset every frame unless a specific toggle is activated so that weapons don't destroy themselves. This required a slight little hack to make work for Targeting Beams.

There are a couple places in beam.cpp where I added checks for a floating and targeting beam. Normal_fire beams is one of them. I'm not 100% sure if those checks are valid. They seem to be but if not I can change them to a direct game state check instead just to get those beams working for the Lab specifically.

Rendering Flags have been added to Weapons and the new Raw Pof objects so that the render options menu in the Lab works correctly for all object types. There is some duplication of flags here now across Ship, Weapons, and Raw. The bloat isn't ideal and I did look into merging them all into an Object->render_flags Flagset member but that would take a significant number of changes for Ships specifically. It's doable, and preferable even, but given the scope of the change should be its own separate PR. So for now I've opted for Weapons and Raw to follow the styling of Ships.

@wookieejedi wookieejedi added enhancement A new feature or upgrade of an existing feature to add additional functionality. Lab A feature or bug related to the F3 lab labels May 1, 2025
@MjnMixael MjnMixael added this to the Release 25.0 milestone May 2, 2025
@MjnMixael MjnMixael mentioned this pull request May 5, 2025

@JohnAFernandez JohnAFernandez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So I may just be rust or crazy, but I didn't see anything that needed to be changed in the code that I viewed. Maybe someone else will want to take a look, but adding an object type is less invasive than it would seem.

However, I know there are some if statements that check for object type, and there are quite a few switch statements. Before I could approve, I would just ask that you double check at least the switch statements.

@MjnMixael

Copy link
Copy Markdown
Contributor Author

So I may just be rust or crazy, but I didn't see anything that needed to be changed in the code that I viewed. Maybe someone else will want to take a look, but adding an object type is less invasive than it would seem.

However, I know there are some if statements that check for object type, and there are quite a few switch statements. Before I could approve, I would just ask that you double check at least the switch statements.

I remember also being rather surprised at how easy it was. In my initial research on adding a new type I found that most object type switch states don't check for every single type and will usually Assert or Int3() in their default case for illegal object types while a few will gracefully do nothing depending on the context.

I went through them back then and just over the last 20 minutes as requested. I'm comfortable with what I see, relying on the illegal cases to handle it as designed. For example, if the object sound code is running on a RAW_POF type it will currently int3() in the default case. I know we prefer assertions to int3s but if I go changing all the int3s related to the object type code we'd be here all year. My main point for this example is RAW_POF cannot currently have sound so if we somehow end up there, then the default case is the correct one to run.

This PR currently only adds RAW_POF cases to switch statements and if statements that relate to model rendering since the only way the type can exist right now is in the lab for rendering. If RAW_POF is expanded in the future to end up in actual missions then that PR will need to go much further in addressing the many code branches it could end up in. (Sound, Targeting, Radar, Collisions, to name a few). For now though, from what I can see, I trust those branches to throw up the appropriate red flags for an invalid object type along those code paths.

On the rendering side for the object type, I've tested it with many different mission backgrounds loaded from retail, MVPs, and BtA and couldn't find a case that wasn't covered. I'm fairly confident that I found all the rendering branches because I matched places where the OBJ_SHIP type is used.

@Baezon Baezon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks pretty good to me too, just a few small points.

Comment thread code/object/object.cpp Outdated
Comment thread code/object/object.cpp Outdated
Comment thread code/weapon/beam.cpp Outdated
Comment thread code/weapon/beam.cpp Outdated
@wookieejedi

Copy link
Copy Markdown
Member

One thought, just to make spurn discussion
Weapon life is reset every frame unless a specific toggle is activated so that weapons don't destroy themselves

This makes sense, though if a weapon uses curves based on lifetime, the lab won't display it accurately. For example, if the weapon is a laser with a lifetime curve that controls brightness. If the lifetime is always reset to the full life, then the brightness will always be the initial brightness and won't change. I'm not entirely sure how or if this could be effectively dealt with...
An alternative that might be worth investigating is resetting the weapon lifetime only when the lifetime is about to expire, but this is probably tricky...In any case, just wanted to bring it up as a point now rather than later :)

@MjnMixael

Copy link
Copy Markdown
Contributor Author

One thought, just to make spurn discussion
Weapon life is reset every frame unless a specific toggle is activated so that weapons don't destroy themselves

This makes sense, though if a weapon uses curves based on lifetime, the lab won't display it accurately. For example, if the weapon is a laser with a lifetime curve that controls brightness. If the lifetime is always reset to the full life, then the brightness will always be the initial brightness and won't change. I'm not entirely sure how or if this could be effectively dealt with...
An alternative that might be worth investigating is resetting the weapon lifetime only when the lifetime is about to expire, but this is probably tricky...In any case, just wanted to bring it up as a point now rather than later :)

That's why there's a toggle to allow weapon life to extinguish. I don't think there's a better way. Whatever version you choose, you'll be resetting at some point. Doing it at the first frame seems to be the least confusing because you don't get partial versions of effects like that. The weapon is static, relatively speaking, or gets a normal life cycle.

@MjnMixael

Copy link
Copy Markdown
Contributor Author

Further thought on the weapon life thing.

First, I changed the toggle button from "Allow weapon to reach end of life" to "Progress weapon lifetime" which is a little more clear that it does more than just let the weapon expire.

Second, I also thought about some kind of auto-respawn but decided against it. There's just too much that can happen at weapon end-of-life to try and detect when it's all finished in order to respawn the weapon.

I'm happy with this how it is. If you want to test things with the weapon that rely on the lifetime timer then there is a toggle for that available. Respawning has also been adjusted so that you can just click on the weapon again rather than having to click to a different weapon class and back.

@MjnMixael MjnMixael force-pushed the lab_weapon_viewer branch from 913caf4 to e495c3e Compare May 25, 2025 17:11

@JohnAFernandez JohnAFernandez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really appreciate you going with the request, and being really careful. Yes, I don't think it's your job to chase down those int3s right now.

@Baezon

Baezon commented May 25, 2025

Copy link
Copy Markdown
Member

Fwiw, if one wanted to scrutinize lifetime curves in the lab, I think some form of time compression would also be helpful (in a different PR).

@wookieejedi

Copy link
Copy Markdown
Member

Yeah, that all sounds good, thanks!

@wookieejedi wookieejedi merged commit 5078826 into scp-fs2open:master May 26, 2025
16 checks passed
@MjnMixael MjnMixael deleted the lab_weapon_viewer branch May 26, 2025 13:36
Kestrellius pushed a commit to Kestrellius/fs2open.github.com that referenced this pull request May 26, 2025
* Activate Lab Weapon viewer

* Allow displaying weapon tech models

* check for a valid tech model when switching

* enable render options for weapons and tech models

* Some cleanup

* Clang

* address feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement A new feature or upgrade of an existing feature to add additional functionality. Lab A feature or bug related to the F3 lab

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants