Closed igromanru closed 1 month ago
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
.
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 ?
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.
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 forGetEngine()
, shouldEngine
be used instead ofGameEngine
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.
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 forController.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 useUGameInstance.LocalPlayers[1].PlayerController
. The local player is always on index 0 of theLocalPlayers
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 logicGetPlayerController
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.
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)
The changelog needs to be updated.
I have also seen games use AController rather than PlayerController for the actual controller, so I do not want to change that.
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?
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.
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()
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
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.
Tested in Everspace 2. In menu:
playerController: BP_PlayerController_C /Game/Maps/Map_MainMenu.Map_MainMenu:PersistentLevel.BP_PlayerController_C_2145820180
player: BP_MainMenuPawn_C /Game/Maps/Map_MainMenu.Map_MainMenu:PersistentLevel.BP_MainMenuPawn_C_2145819395
gameEngine: GameEngine /Engine/Transient.GameEngine_2147482467
gameViewportClient: GameViewportClient /Engine/Transient.GameEngine_2147482467:GameViewportClient_2147480061
world: World /Game/Maps/Map_MainMenu.Map_MainMenu
WorldDeltaSeconds: 0.016665998846292
persistentLevel: Level /Game/Maps/Map_MainMenu.Map_MainMenu:PersistentLevel
worldSettings: ESWorldSettings /Game/Maps/Map_MainMenu.Map_MainMenu:PersistentLevel.ESWorldSettings
gameModeBase: BP_GameModeMainMenu_C /Game/Maps/Map_MainMenu.Map_MainMenu:PersistentLevel.BP_GameModeMainMenu_C_2145820225
gameStateBase: GameState /Game/Maps/Map_MainMenu.Map_MainMenu:PersistentLevel.GameState_2145820185
In game
playerController: BP_PlayerController_C /Game/Maps/Locations/S01L01.S01L01:PersistentLevel.BP_PlayerController_C_2145863052
player: BP_Ship_Player_C /Game/Maps/Locations/S01L01.S01L01:PersistentLevel.BP_Ship_Player_C_2145862214
gameEngine: GameEngine /Engine/Transient.GameEngine_2147482467
gameViewportClient: GameViewportClient /Engine/Transient.GameEngine_2147482467:GameViewportClient_2147480061
world: World /Game/Maps/Locations/S01L01.S01L01
WorldDeltaSeconds: 0.016727700829506
persistentLevel: Level /Game/Maps/Locations/S01L01.S01L01:PersistentLevel
worldSettings: ESWorldSettings /Game/Maps/Locations/S01L01.S01L01:PersistentLevel.ESWorldSettings
gameModeBase: BP_GameModeBase_C /Game/Maps/Locations/S01L01.S01L01:PersistentLevel.BP_GameModeBase_C_2145864454
gameStateBase: GameState /Game/Maps/Locations/S01L01.S01L01:PersistentLevel.GameState_2145864099
I don't know where to find a UE4 or 5 game which uses only AController as base for it's player controller.
It turns out that nobody uses UE for racing games because the in-built physics is not suitable for it and you still need something of your own.
Everspace 2 uses a Pawn that is not ACharacter based but still uses APlayerController as base.
Tested in Satisfactory.
Menu
playerController: FGPlayerControllerBase /Game/FactoryGame/Map/MenuScenes/Map_Menu_1_0.Map_Menu_1_0:PersistentLevel.FGPlayerControllerBase_2147482147
player: DefaultPawn /Game/FactoryGame/Map/MenuScenes/Map_Menu_1_0.Map_Menu_1_0:PersistentLevel.DefaultPawn_2147482136
gameEngine: FGGameEngine /Engine/Transient.FGGameEngine_2147482619
gameViewportClient: FGGameViewportClient /Engine/Transient.FGGameEngine_2147482619:FGGameViewportClient_2147482499
world: World /Game/FactoryGame/Map/MenuScenes/Map_Menu_1_0.Map_Menu_1_0
WorldDeltaSeconds: 0.0069451034069061
persistentLevel: Level /Game/FactoryGame/Map/MenuScenes/Map_Menu_1_0.Map_Menu_1_0:PersistentLevel
worldSettings: FGWorldSettings /Game/FactoryGame/Map/MenuScenes/Map_Menu_1_0.Map_Menu_1_0:PersistentLevel.FGWorldSettings
gameModeBase: BP_GameModeMenu_C /Game/FactoryGame/Map/MenuScenes/Map_Menu_1_0.Map_Menu_1_0:PersistentLevel.BP_GameModeMenu_C_2147482204
gameStateBase: FGMainMenuState /Game/FactoryGame/Map/MenuScenes/Map_Menu_1_0.Map_Menu_1_0:PersistentLevel.FGMainMenuState_2147482171
After loading my save file
playerController: BP_PlayerController_C /Game/FactoryGame/Map/GameLevel01/Persistent_Level.Persistent_Level:PersistentLevel.BP_PlayerController_C_2147476792
player: Char_Player_C /Game/FactoryGame/Map/GameLevel01/Persistent_Level.Persistent_Level:PersistentLevel.Char_Player_C_2143067122
gameEngine: FGGameEngine /Engine/Transient.FGGameEngine_2147482619
gameViewportClient: FGGameViewportClient /Engine/Transient.FGGameEngine_2147482619:FGGameViewportClient_2147482499
world: World /Game/FactoryGame/Map/GameLevel01/Persistent_Level.Persistent_Level
WorldDeltaSeconds: 0.00694540143013
persistentLevel: Level /Game/FactoryGame/Map/GameLevel01/Persistent_Level.Persistent_Level:PersistentLevel
worldSettings: FGWorldSettings /Game/FactoryGame/Map/GameLevel01/Persistent_Level.Persistent_Level:PersistentLevel.FGWorldSettings
gameModeBase: BP_GameMode_C /Game/FactoryGame/Map/GameLevel01/Persistent_Level.Persistent_Level:PersistentLevel.BP_GameMode_C_2147477551
gameStateBase: BP_GameState_C /Game/FactoryGame/Map/GameLevel01/Persistent_Level.Persistent_Level:PersistentLevel.BP_GameState_C_2147477336
Remnant 2: In main menu:
playerController: BP_MenuPlayerController_C /Game/Maps/Menus/MainMenu.MainMenu:PersistentLevel.BP_MenuPlayerController_C_2147482446
player: Pawn /Game/Maps/Menus/MainMenu.MainMenu:PersistentLevel.Pawn_2147482435
gameEngine: GameEngine /Engine/Transient.GameEngine_2147482619
gameViewportClient: GameViewportClientGunfire /Engine/Transient.GameEngine_2147482619:GameViewportClientGunfire_2147482509
world: World /Game/Maps/Menus/MainMenu.MainMenu
WorldDeltaSeconds: 0.016891699284315
persistentLevel: Level /Game/Maps/Menus/MainMenu.MainMenu:PersistentLevel
worldSettings: WorldSettings /Game/Maps/Menus/MainMenu.MainMenu:PersistentLevel.WorldSettings
gameModeBase: GameMode_Menu_C /Game/Maps/Menus/MainMenu.MainMenu:PersistentLevel.GameMode_Menu_C_2147482458
gameStateBase: GameState /Game/Maps/Menus/MainMenu.MainMenu:PersistentLevel.GameState_2147482450
In game:
playerController: Remnant_PlayerController_C /Game/Maps/Main.Main:PersistentLevel.Remnant_PlayerController_C_2147481394
player: Character_Master_Player_C /Game/Maps/Main.Main:PersistentLevel.Character_Master_Player_C_2147480669
gameEngine: GameEngine /Engine/Transient.GameEngine_2147482619
gameViewportClient: GameViewportClientGunfire /Engine/Transient.GameEngine_2147482619:GameViewportClientGunfire_2147482509
world: World /Game/Maps/Main.Main
WorldDeltaSeconds: 0.016253501176834
persistentLevel: Level /Game/Maps/Main.Main:PersistentLevel
worldSettings: WorldSettings /Game/Maps/Main.Main:PersistentLevel.WorldSettings
gameModeBase: TPSGameMode_Main_C /Game/Maps/Main.Main:PersistentLevel.TPSGameMode_Main_C_2147481407
gameStateBase: TPSGameState_C /Game/Maps/Main.Main:PersistentLevel.TPSGameState_C_2147481399
The PR now requires #666 to be merged first.
I think this can be merged as soon as #666 has been merged, I think we've done enough testing.
Just tested with zDEV-UE4SS_v3.0.1-175-g229040b aka. the latest version from 229040b. Added this code in addiiton to the lua code from above.
local invalidObject = CreateInvalidObject()
print("invalidObject variable type: " .. type(invalidObject))
if invalidObject then
print("invalidObject userdata type: " .. invalidObject:type())
print("invalidObject IsValid:" .. tostring(invalidObject:IsValid()))
end
Result:
[17:24:02.7833272] [Lua] invalidObject variable type: userdata
[17:24:02.7833350] [Lua] invalidObject userdata type: UObject
[17:24:02.7833424] [Lua] invalidObject IsValid:false
[17:24:02.7946428] [Lua] playerController: Abiotic_PlayerController_C /Game/Maps/MainMenu.MainMenu:PersistentLevel.Abiotic_PlayerController_C_2147482346
[17:24:02.7946555] [Lua] player invalid
[17:24:02.8032284] [Lua] gameEngine: GameEngine /Engine/Transient.GameEngine_2147482617
[17:24:02.8032463] [Lua] gameViewportClient: AbioticGameViewportClient /Engine/Transient.GameEngine_2147482617:AbioticGameViewportClient_2147482417
[17:24:02.8032689] [Lua] world: World /Game/Maps/MainMenu.MainMenu
[17:24:02.8032926] [Lua] WorldDeltaSeconds: 0.016244702041149
[17:24:02.8033039] [Lua] persistentLevel: Level /Game/Maps/MainMenu.MainMenu:PersistentLevel
[17:24:02.8033182] [Lua] worldSettings: AbioticWorldSettings /Game/Maps/MainMenu.MainMenu:PersistentLevel.AbioticWorldSettings
[17:24:02.8033321] [Lua] gameModeBase: Abiotic_MainMenu_GameMode_C /Game/Maps/MainMenu.MainMenu:PersistentLevel.Abiotic_MainMenu_GameMode_C_2147482370
[17:24:02.8033440] [Lua] gameStateBase: GameState /Game/Maps/MainMenu.MainMenu:PersistentLevel.GameState_2147482353
Description UEHelpers changes:
Type of change
How Has This Been Tested?
Output: In manu:
In game:
Checklist
Additional information I would like to discuss the changes and get feedback. I am particularly unsure about the version number.