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

UEHelpers improvements. Ensure that all functions return a RemoteObject and not nil. Add more functions, Rework some #650

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

igromanru
Copy link

@igromanru igromanru commented Sep 7, 2024

Description
Annotation fix in Types.lua:

  • Removed ? from the return annotation of FindFirstOf. It never returns a nil, always an object

UEHelpers changes:

  • Bumped version to 3. (I don't think it's really necessary since no breaking changes were made. Also should it be v3 or something like 2.1 etc.?)
  • Added a local RemoteObject class that allows to create placeholder objects with a IsValid function
  • Reworked UEHelpers to ensure that all functions never return nil but an object with a IsValid function
  • Added functions GetEngine(), GetGameViewportClient()
  • Reworked GetWorld() to get the current UWorld instance from UGameViewportClient. (PlayerController can be invalid, UGameViewportClient always exist)
  • Rework GetWorldContextObject() to return UGameViewportClient. (Same as for GetWorld(). In UE any UObject that has the GetWorld() function is a valid WorldContext)
  • Removed duplicate function GetKismetMathLibrary()
  • Added functions GetKismetStringLibrary and GetKismetTextLibrary
  • Added annotations for Lua Server
  • Added GetGameInstance() function
  • Added GetPlayer() function to get local player pawn
  • Reworked GetPlayerController(), to searches for the first AController with bPlayerController flag
  • Added FName utility functions to simply Find or Add a FName

Type of change

  • Lua annotation changes
  • New features (non-breaking change which adds functionality)

How Has This Been Tested?

local playerController = UEHelpers.GetPlayerController()
if playerController:IsValid() then
	LogDebug("playerController: " .. playerController:GetFullName())
else
	LogDebug("playerController invalid")
end
local player = UEHelpers.GetPlayer()
if player:IsValid() then
	LogDebug("player: " .. player:GetFullName())
else
	LogDebug("player invalid")
end
local gameEngine = UEHelpers.GetEngine()
if gameEngine:IsValid() then
	LogDebug("gameEngine: " .. gameEngine:GetFullName())
end
local gameViewportClient = UEHelpers.GetGameViewportClient()
if gameViewportClient:IsValid() then
	LogDebug("gameViewportClient: " .. gameViewportClient:GetFullName())
end
local world = UEHelpers.GetWorld()
if world:IsValid() then
	LogDebug("world: " .. world:GetFullName())
end
LogDebug("WorldDeltaSeconds: " .. UEHelpers.GetGameplayStatics():GetWorldDeltaSeconds(UEHelpers.GetWorldContextObject()))
local persistentLevel = UEHelpers.GetPersistentLevel()
if persistentLevel:IsValid() then
	LogDebug("persistentLevel: " .. persistentLevel:GetFullName())
else
	LogDebug("persistentLevel invalid")
end
local worldSettings = UEHelpers.GetWorldSettings()
if worldSettings:IsValid() then
	LogDebug("worldSettings: " .. worldSettings:GetFullName())
else
	LogDebug("worldSettings invalid")
end
local gameModeBase = UEHelpers.GetGameModeBase()
if gameModeBase:IsValid() then
	LogDebug("gameModeBase: " .. gameModeBase:GetFullName())
else
	LogDebug("gameModeBase invalid")
end
local gameStateBase = UEHelpers.GetGameStateBase()
if gameStateBase:IsValid() then
	LogDebug("gameStateBase: " .. gameStateBase:GetFullName())
else
	LogDebug("gameStateBase invalid")
end

Output:
In manu:

playerController: Abiotic_PlayerController_C /Game/Maps/MainMenu.MainMenu:PersistentLevel.Abiotic_PlayerController_C_2147482360
player invalid
gameEngine: GameEngine /Engine/Transient.GameEngine_2147482617
gameViewportClient: AbioticGameViewportClient /Engine/Transient.GameEngine_2147482617:AbioticGameViewportClient_2147482429
world: World /Game/Maps/MainMenu.MainMenu
WorldDeltaSeconds: 0.016670398414135
persistentLevel: Level /Game/Maps/MainMenu.MainMenu:PersistentLevel
worldSettings: AbioticWorldSettings /Game/Maps/MainMenu.MainMenu:PersistentLevel.AbioticWorldSettings
gameModeBase: Abiotic_MainMenu_GameMode_C /Game/Maps/MainMenu.MainMenu:PersistentLevel.Abiotic_MainMenu_GameMode_C_2147482384
gameStateBase: GameState /Game/Maps/MainMenu.MainMenu:PersistentLevel.GameState_2147482367

In game:

playerController: Abiotic_PlayerController_C /Game/Maps/Facility.Facility:PersistentLevel.Abiotic_PlayerController_C_2147473707
player: Abiotic_PlayerCharacter_C /Game/Maps/Facility.Facility:PersistentLevel.Abiotic_PlayerCharacter_C_2147473696
gameEngine: GameEngine /Engine/Transient.GameEngine_2147482617
gameViewportClient: AbioticGameViewportClient /Engine/Transient.GameEngine_2147482617:AbioticGameViewportClient_2147482429
world: World /Game/Maps/Facility.Facility
WorldDeltaSeconds: 0.0
persistentLevel: Level /Game/Maps/Facility.Facility:PersistentLevel
worldSettings: AbioticWorldSettings /Game/Maps/Facility.Facility:PersistentLevel.AbioticWorldSettings
gameModeBase: Abiotic_Survival_GameMode_C /Game/Maps/Facility.Facility:PersistentLevel.Abiotic_Survival_GameMode_C_2147473919
gameStateBase: Abiotic_Survival_GameState_C /Game/Maps/Facility.Facility:PersistentLevel.Abiotic_Survival_GameState_C_214747374

Checklist

  • I have commented my code, particularly in hard-to-understand areas.

Additional information
I would like to discuss the changes and get feedback.
I am particularly unsure about the version number.

…ith a IsValid function

feat: Rework UEHelpers to ensure that all functions never return nil but an object with a IsValid function
feat: Rework GetPlayerController():
- PlayerControllers are never just AController, no need to use FindAllOf("Controller") if no APlayerControlle instances exist
- If "PlayerControllers" is empty (no APlayerController instanced were found) we should return an object
- Remove redundant "pairs(PlayerControllers or {})", we check already if PlayerControllers is nil before
- ipairs is slightly faster than pairs and preferable for arrays (tables with numeric keys)
- Remove redundant PlayerController IsValid check at the end. PlayerControllers from FindAllOf should all be valid, otherwise the user have to check IsValid on the return anyway
feat: Add funtion GetGameEngine(), GetGameViewportClient() and GetWorld()
- An instance of UGameEngine, UGameViewportClient and UWorld exists at all time, while PlayerController doesn't exist in main menu of some games
feat: Rework GetWorldContextObject() to return UGameViewportClient. (In UE any UObject that has the GetWorld() function is a valid WorldContext)
feat: UEHelpers: Add functions GetKismetStringLibrary and GetKismetTextLibrary
docs: UEHelpers: Add annotations for Lua Server
…PlayerControllers array is smaller than 1, in case future UE4SS versions will return only an empty array with FindAllOf
@UE4SS
Copy link
Collaborator

UE4SS commented Sep 7, 2024

The reason we use FindAllOf("Controller") instead of FindAllOf("PlayerController") is that some games don't use APlayerController, but instead provide their own controller based on AController.
This is the case in a racing game I used a while back to do some testing with.
So I still think we should try find all of Controller.

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 7, 2024

Regarding GetWorld(), you've made it rely on ViewportClient which doesn't always exist, for example for Palworld servers.
And for GetEngine(), should Engine be used instead of GameEngine just in case ?

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 7, 2024

I think you should add a check for the existence of RemoteObject before creating it.
That would allow us to implement it in C++ in the future (to expose it to all mods by default) without requiring that people update their UEHelpers file, it's to reduce possible compatibility problems.

@igromanru
Copy link
Author

igromanru commented Sep 7, 2024

I think you should add a check for the existence of RemoteObject before creating it. That would allow us to implement it in C++ in the future (to expose it to all mods by default) without requiring that people update their UEHelpers file, it's to reduce possible compatibility problems.

In Lua variables with the same name overshadows the previous declaration. It shouldn't create any compatibility issues if you expose the RemoteObject from C++ later and remove RemoteObject from the UEHelpers. And since it's a local variable nobody should be able use it in their mods anyway.

Regarding GetWorld(), you've made it rely on ViewportClient which doesn't always exist, for example for Palworld servers. And for GetEngine(), should Engine be used instead of GameEngine just in case ?

Good point, I've no experience with server modding, I'll add a fallback.
Regarding GameEngine, I'm unsure. I haven't seen a single game making own GamaEngine from UEngine, all I've seen (over 20 games) were build upon UGameEngine. I guess its safer to use UEngine. I'll change it.

About APlayerController, from my experience I never seen a game use AController as base, even in RTS games. Do you remember how old the game was and which UE version it used?
My issue with AController is that APlayerController has a lot of important functions and properties which AController is missing.
Also I see a problem with a function that is called GetPlayerController returning a simple Controller instead. It's incoherent.
If there is a game that works differently, it requires a special handling anyway.

Another topic that I would like to discuss. Currently GetPlayerController checks for Controller.Pawn:IsValid() and Controller.Pawn:IsPlayerControlled(), which means that it will return a not valid object in the main menu, while in reality a local PlayerController actually exists, it just has no Pawn to control.
Actually, the best approach to get a playercontroller of the local player is to use UGameInstance.LocalPlayers[1].PlayerController.
The local player is always on index 0 of the LocalPlayers array (index 1 in Lua).
It might be out of the scope for the current RP, but it's actually a better way to get PlayerController of the main local player.
In theory a game with split screen can have multiple local players and will have multiple PlayerControllers. As soon player 1 will die, with the current logic GetPlayerController will return the controller of second player since it has a Pawn.

feat: Add a fallback to GetWorld(), if PlayerController doesn't exists it will try to get UWorld from UGameViewportClient
feat: Use GetWorld() in GetWorldContextObject() instead of GetGameViewportClient() in GetWorldContextObject()
@UE4SS
Copy link
Collaborator

UE4SS commented Sep 7, 2024

About APlayerController, from my experience I never seen a game use AController as base, even in RTS games. Do you remember how old the game was and which UE version it used? My issue with AController is that APlayerController has a lot of important functions and properties which AController is missing. Also I see a problem with a function that is called GetPlayerController returning a simple Controller instead. It's incoherent. If there is a game that works differently, it requires a special handling anyway.

It doesn't really matter about the functions.
If the game does have a PlayerController, it should find it, and it should be used even if we only try to find Controller.
The name does make it kind of strange, but whether the game needs special handling depends on why the mod needed the controller to begin with.
I guess it's probably fine to have it be PlayerController but I just don't think it's a change worth making since as far as I know there were no problems with just using Controller, but obviously let me know if I'm wrong here.

Another topic that I would like to discuss. Currently GetPlayerController checks for Controller.Pawn:IsValid() and Controller.Pawn:IsPlayerControlled(), which means that it will return a not valid object in the main menu, while in reality a local PlayerController actually exists, it just has no Pawn to control. Actually, the best approach to get a playercontroller of the local player is to use UGameInstance.LocalPlayers[1].PlayerController. The local player is always on index 0 of the LocalPlayers array (index 1 in Lua). It might be out of the scope for the current RP, but it's actually a better way to get PlayerController of the main local player. In theory a game with split screen can have multiple local players and will have multiple PlayerControllers. As soon player 1 will die, with the current logic GetPlayerController will return the controller of second player since it has a Pawn.

This is probably fine, and we can always revert it with a minor patch later if it causes some unforeseen consequences.

feat: UEHelpers: Add GetPlayer() function
feat: UEHelpers: Rework GetPlayerController() to get the player controller from first/main local player
@igromanru
Copy link
Author

igromanru commented Sep 7, 2024

I went back to the roots aka. UE4/5 source code and articles. The APlayerController is the class that is meant to be used by human player to control a pawn. AController is abstraction that can be used for NPC/AI.
Unreal Engine itself gets player's PlayerController from LocalPlayers with functions like UGameInstance::GetFirstLocalPlayerController or UGameplayStatics::GetPlayerController.
Therefore I want to propose to go the same route here.
I made code changes and edited first post with descriptions and tests.
New approach allows to get local player controller even without a Pawn to control, which was actually previously an issue that it wasn't possible to get PlayerController and therefore UWorld in main menu or while the player was dead/spectating. (In Discord someone had trouble with it).
It shouldn't cause any comparability issues, since the main reason to use the function was to get the local player controller and unless a game was made in a weird way it normally obliges to UE standards. At least I personally haven't seen a game changing the core Engine since UE 4.18 (6 years ago or something).

The only problem that I see if someone used previous GetPlayerController to get any PlayerController with a Pawn on the server.
Which makes no sense to me. I would still assume to get first player controller, which is always valid even without a Pawn to control. (Pawn can be dead and despawned)

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 8, 2024

The changelog needs to be updated.

@narknon
Copy link
Collaborator

narknon commented Sep 8, 2024

I have also seen games use AController rather than PlayerController for the actual controller, so I do not want to change that.

…Controller for first PlayerController

feat: UEHelpers: Remove fallback to GameViewportClient in GetWorld(), since the new GetPlayerController() should always return a valid controller
@igromanru
Copy link
Author

igromanru commented Sep 8, 2024

If you know and want to support such games, UE4SS must be tested with these customized games.
I changed GetPlayerController() once again, but without a game that doesn't have an APlayerController we can't be sure it work as expected.

Where do the changes belong in the changelog? Extra section for UEHelpers?

Also nobody has commented on version number so far.
Should I keep the UEHelpers version at 3, should it be something else?

- Revert GetWorldContextObject() back to use PlayerController, since now it should always exists
- Add GetPersistentLevel() and GetWorldSettings() functions
- Add GetActorFromHitResult(HitResult) function
@UE4SS
Copy link
Collaborator

UE4SS commented Sep 8, 2024

Where do the changes belong in the changelog? Extra section for UEHelpers?

New functions go in the New->Lua API section.
Changes go in the Changes->Lua API section.
Bug fixes go in the Fixes->Lua API section.
Obviously mention UEHelpers for each entry so that people don't think these are globally available.
You can also combine multiple new things together if you want to, for example: Added <Func1>, <Func2>, and <Func3> to UEHelpers.

Also nobody has commented on version number so far. Should I keep the UEHelpers version at 3, should it be something else?

Version 3 is fine.

igromanru added a commit to igromanru/RE-UE4SS that referenced this pull request Sep 8, 2024
… ForceInvalidateCache parameter as optional

feat: UEHelpers: Added functions GetGameModeBase() and GetGameStateBase()
docs: Added Changelog from UE4SS-REGH-650
igromanru added a commit to igromanru/RE-UE4SS that referenced this pull request Sep 8, 2024
…annotated ForceInvalidateCache parameter as optional

feat: UEHelpers: Added functions GetGameModeBase() and GetGameStateBase()
docs: Added changes from UE4SS-REGH-650 to the Changelog
…annotated ForceInvalidateCache parameter as optional

feat: UEHelpers: Added functions GetGameModeBase() and GetGameStateBase()
docs: Added changes from UE4SS-REGH-650 to the Changelog
@igromanru
Copy link
Author

More useful getter functions added. Changelog updated.
I think I'm now done.
I would ask you guys to review all the changes and see if you're happy with latest GetPlayerController()

@igromanru
Copy link
Author

Just tested in another game (LOLLIPOP CHAINSAW RePOP, UE v5.3.1) to have more games covered.
In main menu:

playerController: PlayerController /Game/Lollipop/Level/start/MENU_PL.MENU_PL:PersistentLevel.PlayerController_2147357341
player: BP_2DOnlyPlayer_C /Game/Lollipop/Level/start/MENU_PL.MENU_PL:PersistentLevel.BP_2DOnlyPlayer_C_2147357333
gameEngine: GameEngine /Engine/Transient.GameEngine_2147482621
gameViewportClient: GameViewportClient /Engine/Transient.GameEngine_2147482621:GameViewportClient_2147482531
world: World /Game/Lollipop/Level/start/MENU_PL.MENU_PL
WorldDeltaSeconds: 0.016725201159716
persistentLevel: Level /Game/Lollipop/Level/start/MENU_PL.MENU_PL:PersistentLevel
worldSettings: WorldSettings /Game/Lollipop/Level/start/MENU_PL.MENU_PL:PersistentLevel.WorldSettings
gameModeBase: BP_2DOnlyGameMode_C /Game/Lollipop/Level/start/MENU_PL.MENU_PL:PersistentLevel.BP_2DOnlyGameMode_C_2147357372
gameStateBase: GameStateBase /Game/Lollipop/Level/start/MENU_PL.MENU_PL:PersistentLevel.GameStateBase_2147357346

In map screen

playerController: PlayerController /Game/Lollipop/Level/start/RESULT_PL.RESULT_PL:PersistentLevel.PlayerController_2147358508
player: BP_2DOnlyPlayer_C /Game/Lollipop/Level/start/RESULT_PL.RESULT_PL:PersistentLevel.BP_2DOnlyPlayer_C_2147358500
gameEngine: GameEngine /Engine/Transient.GameEngine_2147482621
gameViewportClient: GameViewportClient /Engine/Transient.GameEngine_2147482621:GameViewportClient_2147482531
world: World /Game/Lollipop/Level/start/RESULT_PL.RESULT_PL
WorldDeltaSeconds: 0.016495998948812
persistentLevel: Level /Game/Lollipop/Level/start/RESULT_PL.RESULT_PL:PersistentLevel
worldSettings: WorldSettings /Game/Lollipop/Level/start/RESULT_PL.RESULT_PL:PersistentLevel.WorldSettings
gameModeBase: BP_2DOnlyGameMode_C /Game/Lollipop/Level/start/RESULT_PL.RESULT_PL:PersistentLevel.BP_2DOnlyGameMode_C_2147358539
gameStateBase: GameStateBase /Game/Lollipop/Level/start/RESULT_PL.RESULT_PL:PersistentLevel.GameStateBase_2147358513

In game:

playerController: BP_PlayerController_C /Game/Lollipop/Level/Stage00/Stage00_PL.Stage00_PL:PersistentLevel.BP_PlayerController_C_2147479865
player: BP_CH_Main_Juliet_C /Game/Lollipop/Level/Stage00/Stage00_PL.Stage00_PL:PersistentLevel.BP_CH_Main_Juliet_C_2147479857
gameEngine: GameEngine /Engine/Transient.GameEngine_2147482621
gameViewportClient: GameViewportClient /Engine/Transient.GameEngine_2147482621:GameViewportClient_2147482531
world: World /Game/Lollipop/Level/Stage00/Stage00_PL.Stage00_PL
WorldDeltaSeconds: 0.016574300825596
persistentLevel: Level /Game/Lollipop/Level/Stage00/Stage00_PL.Stage00_PL:PersistentLevel
worldSettings: WorldSettings /Game/Lollipop/Level/Stage00/Stage00_PL.Stage00_PL:PersistentLevel.WorldSettings_1
gameModeBase: 3DGameMode_C /Game/Lollipop/Level/Stage00/Stage00_PL.Stage00_PL:PersistentLevel.3DGameMode_C_2147480098
gameStateBase: GameStateBase /Game/Lollipop/Level/Stage00/Stage00_PL.Stage00_PL:PersistentLevel.GameStateBase_2147479879

Copy link
Collaborator

@UE4SS UE4SS left a comment

Choose a reason for hiding this comment

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

I have yet to test this PR.

assets/Mods/shared/UEHelpers/UEHelpers.lua Outdated Show resolved Hide resolved
@igromanru
Copy link
Author

igromanru commented Sep 21, 2024

It slipped my mind that the whole Types.lua is purely a meta file.
It goes a bit beyond the original PR context, but I've now added definitions of NAME_None, FName overloads with FindType parameter and even the EFindName enum to the Types.lua.
Lua Server now properly detects these functions and variables.

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.

3 participants