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

AngelScript: fixed ref counting, added ProceduralRoad API #2930

Closed
wants to merge 9 commits into from

Conversation

ohlidalp
Copy link
Member

@ohlidalp ohlidalp commented Aug 6, 2022

Status (last update 2022-10-14)

Many feats originally developed here got merged in #2931.
The object ref-counting system, originally created here, got perfected in it's own repo: https://github.com/only-a-ptr/RefCountingObject-AngelScript

New script features:

  • ref types: TerrainClass, ProceduralRoadClass, ProceduralObjectClass, ProceduralManagerClass, ProceduralPointClass
  • enums: RoadType, TextureFit

Reference counting fix

Previously, if a script kept a pointer (a 'handle' in AngelScript terminology) to BeamClass or VehicleAIClass after the vehicle was removed, it would crash. I studied how the reference counting system works and added a RefCountingObject base class which lets us keep track of the object both inside and outside of the script. It will help registering more objects to the script engine.

@ohlidalp ohlidalp marked this pull request as ready for review August 6, 2022 23:20
@ohlidalp ohlidalp marked this pull request as draft August 7, 2022 18:45
@ohlidalp ohlidalp force-pushed the refcounted-object branch from b9eb90d to b5b4c2c Compare August 7, 2022 23:22
@ohlidalp
Copy link
Member Author

ohlidalp commented Aug 10, 2022

Sneak preview of what's coming. This script tests the new procedural road APIs and will become an useful tool later. The window only appears in terrain editing mode (hotkey Ctrl+Y).
terrain_editor-alpha1.as.zip
I'm not committing the script until final, you can load it manually via RoR.cfg app_custom_scripts=terrain_editor.as
obrazek

@ohlidalp ohlidalp changed the title Fixed AngelScript reference counting pointers. AngelScript: Added terrain and road API, fixed ref counting Aug 10, 2022
@ohlidalp ohlidalp force-pushed the refcounted-object branch 2 times, most recently from ae54ab2 to 06563bc Compare August 12, 2022 18:34
@ohlidalp
Copy link
Member Author

It's all drawn by the script!
terrain_editor-alpha2.as.zip

obrazek

@tritonas00
Copy link
Collaborator

tritonas00 commented Aug 14, 2022

getting some gcc warnings:

/home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/RefCountingObject.h:100:5: warning: ‘void operator delete(void*)’ called on pointer ‘*(RoR::Actor**)((char*)__first)’ with nonzero offset 8 [-Wfree-nonheap-object]
  100 |     }

Hm, can't load any script:

14:39:24: ScriptEngine initializing ...
14:39:24: Type registrations done. If you see no error above everything should be working
14:39:24:  (0, 0): Info = void ProceduralObjectClass::addPoint(procedural_point)
14:39:24:  (0, 0): Error = Don't support passing type 'procedural_point' by value to application in native calling convention on this platform
14:39:24:  (0, 0): Info = void ProceduralObjectClass::insertPoint(int, procedural_point)
14:39:24:  (0, 0): Error = Don't support passing type 'procedural_point' by value to application in native calling convention on this platform
14:39:28:  (0, 0): Error = Invalid configuration. Verify the registered application interface.
14:39:28:  (0, 0): Error = Invalid configuration. Verify the registered application interface.
14:39:53:  (0, 0): Error = Invalid configuration. Verify the registered application interface.

@ohlidalp
Copy link
Member Author

ohlidalp commented Aug 14, 2022

free-nonheap-object? I didn't intend that, must double check. Might be a false alarm, tho: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54202

I didn't expect a problem with calling conventions... But apparently linux x64 is more complicated: https://www.gamedev.net/forums/topic/712220-native-calling-on-linux-x86-64-not-working-for-a-type/

@ohlidalp ohlidalp force-pushed the refcounted-object branch 2 times, most recently from 801c8e9 to 2bb223b Compare August 17, 2022 23:37
@ohlidalp ohlidalp marked this pull request as ready for review August 19, 2022 09:57
@ohlidalp ohlidalp changed the title AngelScript: Added terrain and road API, fixed ref counting AngelScript: Added terrain and road API, monitor UI, loadscript command, fixed ref counting Aug 19, 2022
@ohlidalp ohlidalp changed the title AngelScript: Added terrain and road API, monitor UI, loadscript command, fixed ref counting AngelScript: fixed ref counting, added ProceduralRoad API Oct 12, 2022
@ohlidalp ohlidalp marked this pull request as draft October 12, 2022 23:46
@ohlidalp ohlidalp force-pushed the refcounted-object branch 3 times, most recently from 5ef1f0b to fb602a8 Compare October 14, 2022 02:39
This introduces `RefCountingObject` to the codebase. Using it makes it possible to pass object between C++ and AngelScript without worry. See https://github.com/only-a-ptr/RefCountingObject-AngelScript

In the case of Actor, the refcounting was dummy and the game would certainly crash if a script kept pointer to an Actor which got removed.

Signifficant code changes:
- utils/memory/RefCountingObject* - new files.
- ForwardDeclarations.h - added ActorPtr
- Application.h - updated comments regarding message queue arguments. Instead of sending weak Actor* pointers, a strong ActorPtr* is sent.
- main.cpp - updated message handling to cope with new pointers.
- SimData.h - new entry in `enum class ActorState`: `DISPOSED` - means the actor was removed from simulation and it's component objects were deleted, but it still lives as an empty shell to satisfy existing pointers.
- ActorManager.cpp - DeleteActorInternal() - changed to only dispose, but not delete the actor - the deleting will be done by RefCountingObjectPtr.
- ScriptEngine.h/cpp - added function `setEventsEnabled()` which stops the TRIGGER_EVENT() macros from working, as a hack for fast dirty shutdown.
Signifficant codechanges:
 - global variable `App::g_sim_terrain` removed, replaced by new variable `GameContext::m_terrain`.
 - global function `App::GetSimTerrain()` removed, using `GameContext::GetTerrain()`
 - static factory func `Terrain* Terrain::LoadAndPrepareTerrain()` changed to nonstatic `void Terrain::initialize()`, loading now done by GameContext.
 - Terrain.h: added function `dispose()` and flag `bool m_disposed` - after the terrain is unloaded, the scripts may still hold pointers to it. Therefore it must remain in memory, even though all the component objects were deleted.
Lesson learned for the `RefCountingObject<>` mechanic - implicit conversion of `Foo*` to `FooPtr` is a no-no; the constructor was made private and static function `Bind()` was added to make it obvious creating the shared pointer is an once-per-object operation.
CODECHANGES:
* new file ProceduralRoadAngelscript.cpp - binds ProceduralPointClass, ProceduralRoadClass, ProceduralObjectClass, ProceduralManagerClass
* ProceduralRoad.h/cpp - added strong enums RoadType and TextureFit - extracted from class ProceduralRoad
* TerrainAngelscript.cpp - added getter for procedural manager.
* TObjFileFormat: accomodated new pointer type.
The 'engine' param in `Register*()` funcs was moved to the front, for style:
 - this way it better resembles the AngelScript reg funcs
 - context objects generally get passed first in APIs.
@ohlidalp
Copy link
Member Author

ohlidalp commented Oct 19, 2022

The work is almost done.

The RefCountingObject system works, except there's a breaking nuissance AngelScript side - you can't do game.getPlayerTruck().getTruckName() because you'll get error 'BeamClassPtr has no functon getTruckName()'. You need to do this instead:

BeamClass@ truck = game.getPlayerTruck();
truck.getTruckName(); // now it works!

I updated the terrain editor script. To test, just put it to "Documents/My Games/Rigs of Rods/scripts" and in the game console, say 'loadscript terrain_editor.as'.

terrain_editor-alpha4.as.zip

@RigsOfRods RigsOfRods deleted a comment from AnotherFoxGuy Oct 19, 2022
@RigsOfRods RigsOfRods deleted a comment from tritonas00 Oct 19, 2022
@tritonas00
Copy link
Collaborator

tritonas00 commented Oct 19, 2022

Crashed as soon as i spawned a desperado in f1 testtrack:

Thread 1 "RoR" received signal SIGSEGV, Segmentation fault.
0x000055555b132320 in ?? ()
(gdb) bt
#0  0x000055555b132320 in ?? ()
#1  0x0000555555676d9a in RefCountingObject<RoR::Terrain>::Release (this=0x555557ee2b38)
    at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/memory/RefCountingObject.h:33
#2  RefCountingObject<RoR::Terrain>::Release (this=0x555557ee2b38)
    at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/memory/RefCountingObject.h:28
#3  RefCountingObjectPtr<RoR::Terrain>::ReleaseHandle (this=<optimized out>)
    at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/memory/RefCountingObjectPtr.h:198
#4  RefCountingObjectPtr<RoR::Terrain>::~RefCountingObjectPtr (this=<optimized out>, __in_chrg=<optimized out>)
    at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/memory/RefCountingObjectPtr.h:190
#5  RoR::GfxScene::UpdateScene (this=0x555555c497e0 <RoR::App::g_gfx_scene>, dt_sec=dt_sec@entry=0.378546089)
    at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/gfx/GfxScene.cpp:142
#6  0x00005555555f5cfb in main (argc=<optimized out>, argv=<optimized out>)
    at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/main.cpp:101

Also still got massive spam about:

In destructor ‘RefCountingObject<T>::~RefCountingObject() [with T = RoR::Actor]’,
    inlined from ‘void RefCountingObject<T>::Release() [with T = RoR::Actor]’ at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/memory/RefCountingObject.h:33:13,
    inlined from ‘void RefCountingObject<T>::Release() [with T = RoR::Actor]’ at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/memory/RefCountingObject.h:28:10,
    inlined from ‘void RefCountingObjectPtr<T>::ReleaseHandle() [with T = RoR::Actor]’ at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/memory/RefCountingObjectPtr.h:198:23,
    inlined from ‘RefCountingObjectPtr<T>::~RefCountingObjectPtr() [with T = RoR::Actor]’ at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/memory/RefCountingObjectPtr.h:190:18,
    inlined from ‘void RoR::GfxActor::UpdateParticles(float)’ at /home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/gfx/GfxActor.cpp:533:25:
/home/babis/Downloads/ror-dependencies/rigs-of-rods/source/main/utils/memory/RefCountingObject.h:21:5: warning: ‘void operator delete(void*)’ called on pointer ‘<anonymous>.RefCountingObjectPtr<RoR::Actor>::m_ref’ with nonzero offset 8 [-Wfree-nonheap-object]
   21 |     }

@ohlidalp
Copy link
Member Author

Purpose: To do AngelScript right - currently the object refcounting is dummy and prone to crash.

Estimate: 8 man days. This is hard work but it's an once-and-for-all fix which needs to be done at some point.

Work to be done:

  1. backport changes made here to https://github.com/only-a-ptr/RefCountingObject-AngelScript
  2. replicate the problem there
  3. get help from AngelScript author

@ohlidalp
Copy link
Member Author

I'm closing this because it got stale and it was too invasive to begin with. I'll start a new PR with less code and more added value.

@ohlidalp ohlidalp closed this Nov 16, 2022
@ohlidalp ohlidalp deleted the refcounted-object branch February 28, 2023 09:54
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.

2 participants