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

Hull, armor and shield components #155

Merged
merged 13 commits into from
Feb 18, 2025
Merged

Hull, armor and shield components #155

merged 13 commits into from
Feb 18, 2025

Conversation

royfalk
Copy link
Contributor

@royfalk royfalk commented Jan 31, 2025

  • modify ship_view/upgrade_view to support 4 facet armor and new shield keys
  • Fix bug in ship_view where ypr is 0.
  • Modify minimize_parts script to create new ships.json and units.json
  • Create new ships.json and units.json
  • Fix some stuff in units_old.json

Complements vegastrike/Vega-Strike-Engine-Source#1016

Code Changes:

Issues:

  • *.blank ships have 0 facets and likely can't upgrade shields.
  • Upgrades, such as armor have shield_facets:0

- modify ship_view/upgrade_view to support 4 facet armor and new shield keys
- Fix bug in ship_view where ypr is 0.
- Modify minimize_parts script to create new ships.json and units.json
- Create new ships.json and units.json
- Fix some stuff in units_old.json
Copy link
Contributor

@stephengtuggy stephengtuggy left a comment

Choose a reason for hiding this comment

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

Looks good for the most part. Let's see if we can fix the zero-facet issues before merging this.

Add Afterburner_Upgrade to units_old - prevent overwriting and recreating bug.
Make Llama.begin damaged on start again.
@stephengtuggy stephengtuggy self-requested a review February 3, 2025 01:11
@stephengtuggy stephengtuggy dismissed their stale review February 3, 2025 01:12

Zero-facet issues apparently fixed

@stephengtuggy
Copy link
Contributor

That’s looking quite a bit better. I think I’d like to see these changes get play-tested before we merge them, but I think there’s a good chance that they will pass muster.

I will see if I can play-test this PR myself, once I’m back to a computer.

@kheckwrecker
Copy link

When running Vegastrike in a debugger, I'm seeing a lot of these error messages:

Unit::LoadRow: invalid stof argument trying to parse shield facets
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210FC360.
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210F9F90.
Unit::LoadRow: invalid stof argument trying to parse shield facets
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210FC360.
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210F9F90.
Unit::LoadRow: invalid stof argument trying to parse shield facets
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210FC360.
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210F9F90.
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210FC360.
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210F9F90.
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210FC360.
Exception thrown at 0x00007FF84533B699 in vegastrike-engine.exe: Microsoft C++ exception: std::invalid_argument at memory location 0x000000E4210F9F90.
Unit::LoadRow: invalid stof argument trying to parse shield facets

Will this PR solve these errors? If it might, I can apply this PR and play test it.

@royfalk
Copy link
Contributor Author

royfalk commented Feb 9, 2025

Does your game crash afterward?
I don't think it fixes this. I'm seeing something similar on Linux.

@kheckwrecker
Copy link

Does your game crash afterward? I don't think it fixes this. I'm seeing something similar on Linux.

No, it doesn't crash due to these errors.

without commensurate reactor and capacitors.
@evertvorster
Copy link
Contributor

Indeed, the change to Oswald's shield generator did the trick.

Copy link
Contributor

@stephengtuggy stephengtuggy left a comment

Choose a reason for hiding this comment

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

LGTM

Make radar display cones in degrees and not radians. Wrong units were used.
Make Llama.begin start with some minor damage. Also useful for testing this PR,
Copy link
Contributor

@evertvorster evertvorster left a comment

Choose a reason for hiding this comment

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

This one is good to be merged in my opinion.

@royfalk royfalk merged commit 5c45bb8 into master Feb 18, 2025
11 checks passed
@royfalk royfalk deleted the task_armor branch February 18, 2025 03:52
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.

5 participants