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 unnecessary Destroys #573

Closed
JohnCorby opened this issue Apr 12, 2023 · 5 comments · Fixed by #712
Closed

Remove unnecessary Destroys #573

JohnCorby opened this issue Apr 12, 2023 · 5 comments · Fixed by #712
Assignees
Labels
fixed in dev Instead of closing issues when you fix them in dev just add this. nice to have Features that could be implemented maybe

Comments

@JohnCorby
Copy link
Member

Around the codebase (but mainly in here) we do calls to Destroy when we probably could just be doing SetActive(false)

It would be easier/nicer if we were more consistent with this. Obviously testing required.

@JohnCorby JohnCorby added the nice to have Features that could be implemented maybe label Apr 12, 2023
@JohnCorby JohnCorby self-assigned this Apr 12, 2023
@JohnCorby
Copy link
Member Author

JohnCorby commented Apr 13, 2023

it would also fix the "what." bug presumably wouldnt happen since ForceVolumes wouldnt be destroyed ("what" is caused by a null ForceVolume in ForceDetector, and the only way for that to happen is if the volume was destroyed)

see #596

@FreezeDriedMangos
Copy link
Contributor

I tested removing the destroy call in PlanetDestructionHandler.DisableBody with my current build of Uncertain Futures, in a system that is empty apart from one planet without a surface (and that has one prop loaded from an asset bundle).

No issues were found during this test.

For reference, I simply commented out some of the lines at the end of the function:

            //if (delete)
            //{
            //    GameObject.Destroy(go);
            //}
            //else
            //{
                go.SetActive(false);
                var ol = go.GetComponentInChildren<OrbitLine>();
                if (ol)
                {
                    ol.enabled = false;
                }
            //}

@JohnCorby JohnCorby added the waiting Needs feedback or testing before continuing label Jul 20, 2023
@JohnCorby
Copy link
Member Author

me i will test this at some point with some mods one day eventually and then see if it explodes

@JohnCorby
Copy link
Member Author

according to #670 this apparently breaks shit or something idk

@xen-42
Copy link
Member

xen-42 commented Aug 12, 2023

@JohnCorby its because that PR patched the destroy method so that nothing ever can be destroyed either by base game or by other mods. Doesn't necessarily mean that stopping our code destroying stuff would be bad.

@xen-42 xen-42 mentioned this issue Aug 24, 2023
xen-42 added a commit that referenced this issue Aug 24, 2023
## Improvements
- Cleaned up how planets are destroyed. Should make load screens faster.
Implements #573
@xen-42 xen-42 mentioned this issue Aug 24, 2023
@xen-42 xen-42 removed the waiting Needs feedback or testing before continuing label Aug 24, 2023
xen-42 added a commit that referenced this issue Aug 26, 2023
<!-- A new module or something else important -->
## Major features
-

<!-- A new parameter added to a module, or API feature -->
## Minor features
-

<!-- Some improvement that requires no action on the part of add-on
creators i.e., improved star graphics -->
## Improvements
- Cached more stuff to improve loading times. Implements #683 
- Cleaned up how planets are destroyed. Should make load screens faster.
Implements #573

<!-- Be sure to reference the existing issue if it exists -->
## Bug fixes
- Fixes Bramble colours at a distance. Fixes #372
@xen-42 xen-42 mentioned this issue Aug 26, 2023
@xen-42 xen-42 added the fixed in dev Instead of closing issues when you fix them in dev just add this. label Aug 26, 2023
xen-42 added a commit that referenced this issue Aug 26, 2023
## Major features
- New `ParticleFields` module. Add particles to your planet (ex: leaves,
rain, snow, fireflies, and more). Resolves #219

## Improvements
- Cached more stuff to improve loading times. Resolves #683 
- Cleaned up how planets are destroyed. Should make load screens faster.
Resolves #573

## Bug fixes
- Fixes Bramble colours at a distance. Fixes #372 and #641
- Allow setting ship spawns in main solar system / separately from the
player spawn. Resolves #677
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in dev Instead of closing issues when you fix them in dev just add this. nice to have Features that could be implemented maybe
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants