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

Improve Abstract Instances and FGBuildable #268

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Th3Fanbus
Copy link
Contributor

See individual commit messages for information.

Th3Fanbus added 3 commits July 1, 2024 18:51
At least in the modding environment, trying to use abstract instances
would crash the editor quite often, and compiling blueprints that use
abstract instances would always crash the editor.

Looks like the crashes happen because `StaticManager_PIE` is a global
variable that stores pointers to UObjects, but is not an `UPROPERTY`.
When the GC kills those UObjects, the pointers in the global variable
become dangling pointers without warning.

Make abstract instance managers clean up global state when destroyed,
and use `IsValid()` to avoid returning UObjects that are about to get
killed (as this will result in unpredictable undefined behaviour).

Tested in PIE in the modding environment, this allows seeing abstract
instances in the editor previews (blueprint and level editor) as well
as in PIE worlds. No crashes observed.

FIXME: It is likely that crimes against UE were committed in here...

TODO: There seems to be a bug somewhere that makes abstract instances
in editor worlds get duplicated for some reason, but it does not seem
to cause issues?

NOTE: This also adapts the non-editor code path accordingly, as these
problems could also occur there. However, these changes have not been
validated.

Signed-off-by: Angel Pons <[email protected]>
Be a bit more paranoid when handling abstract instances to avoid NULL
pointer dereferencing. Checking whether `this` is NULL gets optimised
out by most compilers (a NULL `this` is undefined behaviour), so drop
the checks as they are misleading.

Define the `LogBuilding` log category to log a single error, but also
to allow using some inline functions defined in base game headers. If
the log category is not defined, using said inline functions causes a
build-time error.

Signed-off-by: Angel Pons <[email protected]>
Looking at the game's binaries, the root component of buildables has to
default to static mobility. Otherwise, pre-placed buildings do not work
properly in the editor.

The pre-placed buildings in ExampleLevel should no longer be stuck at 0
(which is inside the starting cube), but the level needs to be resaved.

Signed-off-by: Angel Pons <[email protected]>
@Th3Fanbus Th3Fanbus closed this Jul 14, 2024
@Th3Fanbus Th3Fanbus deleted the abstract-instances branch July 14, 2024 20:29
@Th3Fanbus Th3Fanbus restored the abstract-instances branch July 15, 2024 20:19
@Th3Fanbus Th3Fanbus reopened this Jul 15, 2024
@Th3Fanbus Th3Fanbus marked this pull request as draft October 4, 2024 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

1 participant