Skip to content

Fix condition to ignore TransparentFX in pr390 to fix issue #410. - #423

Open
BrettRyland wants to merge 1 commit into
KSPModdingLibs:masterfrom
BrettRyland:master
Open

Fix condition to ignore TransparentFX in pr390 to fix issue #410.#423
BrettRyland wants to merge 1 commit into
KSPModdingLibs:masterfrom
BrettRyland:master

Conversation

@BrettRyland

Copy link
Copy Markdown

PR #390 introduced a condition to ignore TransparentFX when calculating part renderer bounds.

This updates the patch to exclude said renderers. There are cases in the editor where everything is on the TransparentFX layer (detached parts) so we can't do this unconditionally, but it should work in all the cases that matter.

The condition used is insufficient and breaks mods that rely on those bounds for their calculations #410. E.g., BDArmory uses these as part of its hitpoint calculations when the part is created (i.e., while the part is detached).

This PR modifies the condition for when to ignore TransparentFX to take into account detached parts by checking if all parents of the part transform are on the TransparentFX layer.

@Phantomical

Copy link
Copy Markdown
Collaborator

So the problem with the original patch runs a bit deeper than what this approach will fix. The editor will occasionally put everything on the TransparentFX layer and anything that attempts to compute the bounds at that point (and possibly afterwards, not sure) will have issues.

I don't think just adding the check here will work. I do have an approach that I think will work well enough, but the branch for that is on my home computer and I won't be back until later this week. I'll make a PR then.

@BrettRyland

Copy link
Copy Markdown
Author

OK, good to hear that you have another approach that you think will be better.
The additional check that this PR adds was sufficient to fix the issue in my testing with BDArmory, where we need the part size to calculate HP and other details during OnStart or shortly afterwards, during which the part is typically fully on the TransparentFX layer in the editor.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants