EDIT: The actual problem here was with the GetDataDescMap() hook not returning the proper datamap pointer during object destruction. This was because when an object is being destructed, the record is removed, resulting the GetDataDescMap() hook to return the original datamap since the record of the entity is no longer found.
It's still "safe" to use virtual functions in a virtual destructor pre-hook, provided that you are operating on the deepest subclass.
#25 was supposed to address an issue regarding reading from incorrect custom entity datamaps, particularly in an action's OnEnd() callback when invoked by the entity being destroyed (not removed). Although the destruction order is a lot more sane, the error still persists and turns out it was for a different reason. This is because a virtual function is being invoked around the time of an object's destruction, the virtual function in this case being GetDataDescMap().
Calling virtual functions around this time is heavily discouraged. And yes, not even a pre-hook to the virtual destructor is immune to this. When an action's OnEnd() is invoked during this time, there is no guarantee that the entity's vtable pointer remains the same one we hooked, and in a good number of cases it doesn't. As a result, this renders the entity factory's GetDataDescMap() hook ineffective, and any uses of Get/SetEntProp() results in an error.
Therefore, the best thing that can be done is to destroy the Behavior at our earliest convenience: between the time an entity is queued for removal and before the invocation of the object's destructor, yet outside of the Behavior's Update() call (#16). This guarantees the error-free use of Get/SetEntProp() on custom properties in OnEnd().
This is also consistent with how SourcePawn code handles entity cleanup which is typically done when the entity is removed. By contrast, handling entity DESTRUCTION is a completely alien concept for SourcePawn developers, so this change also makes it more intuitive for developers and saves them from having to learn about the C++ quirks of object destruction the hard way.
EDIT: The actual problem here was with the
GetDataDescMap()hook not returning the proper datamap pointer during object destruction. This was because when an object is being destructed, the record is removed, resulting theGetDataDescMap()hook to return the original datamap since the record of the entity is no longer found.It's still "safe" to use virtual functions in a virtual destructor pre-hook, provided that you are operating on the deepest subclass.
#25 was supposed to address an issue regarding reading from incorrect custom entity datamaps, particularly in an action'sOnEnd()callback when invoked by the entity being destroyed (not removed). Although the destruction order is a lot more sane, the error still persists and turns out it was for a different reason. This is because a virtual function is being invoked around the time of an object's destruction, the virtual function in this case beingGetDataDescMap().Calling virtual functions around this time is heavily discouraged. And yes, not even a pre-hook to the virtual destructor is immune to this. When an action'sOnEnd()is invoked during this time, there is no guarantee that the entity's vtable pointer remains the same one we hooked, and in a good number of cases it doesn't. As a result, this renders the entity factory'sGetDataDescMap()hook ineffective, and any uses ofGet/SetEntProp()results in an error.Therefore, the best thing that can be done is to destroy the Behavior at our earliest convenience: between the time an entity is queued for removal and before the invocation of the object's destructor, yet outside of the Behavior'sUpdate()call (#16). This guarantees the error-free use ofGet/SetEntProp()on custom properties inOnEnd().This is also consistent with how SourcePawn code handles entity cleanup which is typically done when the entity is removed. By contrast, handling entity DESTRUCTION is a completely alien concept for SourcePawn developers, so this change also makes it more intuitive for developers and saves them from having to learn about the C++ quirks of object destruction the hard way.Tested to be working on both Windows and Linux.