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

Remove several unused fields from various blueprints #6161

Open
wants to merge 8 commits into
base: deploy/fafdevelop
Choose a base branch
from

Conversation

Basilisk3
Copy link
Contributor

@Basilisk3 Basilisk3 commented May 9, 2024

Description of the proposed changes

This PR removes the following fields from various blueprints:
bp.General.TechLevel
bp.General.Classification
bp.General.Category
bp.General.UnitWeight

Testing done on the proposed changes

All units I tested still functioned normally.

Additional context

According to an older Discord post by @The-Balthazar, these fields can be removed safely.

Checklist

@Basilisk3 Basilisk3 marked this pull request as draft May 9, 2024 13:28
@Basilisk3 Basilisk3 marked this pull request as ready for review May 9, 2024 19:25
Copy link
Member

@Garanas Garanas left a comment

Choose a reason for hiding this comment

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

To solidify #6171 (review), the field Classification is used by the Spooky unit viewer. I then had to fix that by 'guessing' the classification and it's not entirely correct yet:

We already did a 'whoopsie' in #5169 , but point is that even though these values may not be used by the game they can still be read somewhere as they existed for eternity. Please verify if the unit viewers will be fine.

@The-Balthazar
Copy link
Contributor

I have always said that things relying on those fields are prone to the data being wrong because it's not actually used, and I think the fact that Spooky used to have to redefine the values of the Kennels is a prime example of that. But it's still important to make sure things aren't breaking.

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.

None yet

3 participants