-
Notifications
You must be signed in to change notification settings - Fork 147
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
vscript additions and fixes #59
Conversation
baseanimating.h baseanimating.cpp - Added CBaseAnimating::SequenceDuration (ScriptSequenceDuration) - Added CBaseAnimating::GetPlaybackRate - Added CBaseAnimating::SetPlaybackRate - Added CBaseAnimating::GetCycle - Added CBaseAnimating::SetCycle triggers.h triggers.cpp - Fixed CTriggerCamera::ScriptSetFov setting player FOV while disabled - Added CBaseTrigger::Enable - Added CBaseTrigger::Disable - Added CBaseTrigger::TouchTest - Added CBaseTrigger::IsTouching (ScriptIsTouching) vscript_server.nut vscript_server.cpp - Added CEntities::FindByClassnameWithinBox - Added ::SendToConsoleServer - Added ::CancelEntityIOEvent - Added ::GetEntityIOEventTimeLeft - Moved ScriptDispatchParticleEffect to shared code eventqueue.h cbase.h cbase.cpp - Set entity I/O con output colour - Added (CEventQueue::CancelEventsByInput) - Added (CEventQueue::RemoveEvent) - Added (CEventQueue::GetTimeLeft) baseentity.h baseentity.cpp - Added CBaseEntity::SetTransmitState - Added CBaseEntity::GetTransmitState - Added CBaseEntity::AcceptInput (ScriptAcceptInput) - Added CBaseEntity::FireOutput (ScriptFireOutput) - Added CBaseEntity::GetMaxOutputDelay - Added CBaseEntity::CancelEventsByInput - Added player_use event on InputUse - Fixed InputKill on players ivscript.h vscript_squirrel.cpp - Added IScriptVM::ArrayAppend - Fixed buffer overflow crash - Increased print buffer to 2048 from 256 - Set vscript print output colour vscript_funcs_shared.cpp - Added CNetMsgScriptHelper (CScriptGameEventListener) - Added ::ListenToGameEvent - Added ::StopListeningToGameEvent - Added ::StopListeningToAllGameEvents - Added ::FireGameEvent - Added ::FireGameEventLocal (CScriptSaveRestoreUtil) - Added ::SaveTable - Added ::RestoreTable - Added ::ClearSavedTable - Added callbacks ::OnSave, ::OnRestore (CScriptReadWriteFile) - Added ::StringToFile - Added ::FileToString - Added GlobalSys::GetCommandLine - Added GlobalSys::CommandLineCheck - Added GlobalSys::CommandLineCheckStr - Added GlobalSys::CommandLineCheckFloat - Added GlobalSys::CommandLineCheckInt - Added ::GetCPUUsage - Added ::NXPrint - Added ::Msg (ScriptMsg) - Removed misplaced condition checks - Fixed ScriptEntitiesInBox, *AtPoint, *InSphere logic_eventlistener.cpp - Removed redundant dev msg gamestringpool.cpp - Fixed string pool dump sort
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contributing to Mapbase's VScript implementation once again.
Like with your previous PR, I am very impressed and amazed with these changes. I don't see any outstanding problems, but coincidentally, I was already working on an update to Mapbase's VScript which integrated and expanded upon similar features from later branches of Source/Source 2. Fortunately, most of the changes you've made in this PR are based on features I considered experimenting with, but decided would be too difficult for me to replicate in Source 2013 under the scope of the update. I'm really thankful that you've been able to replicate those features yourself. You're almost single-handedly bringing Mapbase's VScript implementation up-to-speed with later branches and that's making me really happy.
I want to mix these changes with my own from the upcoming update, but it's still going to be some time before that update is ready, so I might either merge this PR far in advance or I'll just pull this branch locally and leave the PR itself open until the update is ready.
I've left several comments and questions in this review. There's nothing seriously wrong, but I recommend reading through all of them. Let me know if you think this PR is ready.
@@ -100,6 +100,9 @@ class CBaseAnimating : public CBaseEntity | |||
inline float SequenceDuration( void ) { return SequenceDuration( m_nSequence ); } | |||
float SequenceDuration( CStudioHdr *pStudioHdr, int iSequence ); | |||
inline float SequenceDuration( int iSequence ) { return SequenceDuration(GetModelPtr(), iSequence); } | |||
#ifdef MAPBASE_VSCRIPT | |||
inline float ScriptSequenceDuration( int iSequence ) { return SequenceDuration(GetModelPtr(), iSequence); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would assume this was split into its own function so the inline function could continue to be inlined in its existing uses, but this new function uses inline
anyway. Is that a mistake or am I misunderstanding the purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It exists for the scriptfunc definition, as it does not accept overloaded functions. Inlining is only for consistency, I don't think it matters at all for scriptfuncs.
@@ -131,6 +131,16 @@ BEGIN_DATADESC( CBaseTrigger ) | |||
|
|||
END_DATADESC() | |||
|
|||
#ifdef MAPBASE_VSCRIPT | |||
|
|||
BEGIN_ENT_SCRIPTDESC( CBaseTrigger, CBaseEntity, "Trigger entity" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one of the overlapping changes I mentioned. I was already preparing a CBaseTrigger
script description for the next Mapbase update, although I was also working on a few additional functions:
BEGIN_ENT_SCRIPTDESC( CBaseTrigger, CBaseEntity, "Server-side camera entity" )
DEFINE_SCRIPTFUNC( UsesFilter, "Returns true if this trigger uses a filter." )
DEFINE_SCRIPTFUNC_NAMED( ScriptPassesTriggerFilters, "PassesTriggerFilters", "Returns whether a target entity satisfies the trigger's spawnflags, filter, etc." )
DEFINE_SCRIPTFUNC_NAMED( ScriptIsTouching, "IsTouching", "Returns true if the target entity is touching this trigger." )
DEFINE_SCRIPTFUNC_NAMED( ScriptGetTouchedEntityOfType, "GetTouchedEntityOfType", "Gets the first touching entity which matches the specified class." )
DEFINE_SCRIPTFUNC( PointIsWithin, "Checks if the given vector is within the trigger's volume." )
DEFINE_SCRIPTFUNC( Enable, "Enables this trigger." )
DEFINE_SCRIPTFUNC( Disable, "Disables this trigger." )
DEFINE_SCRIPTFUNC_NAMED( ScriptGetTouchingEntities, "GetTouchingEntities", "Gets all entities touching this trigger (and satisfying its criteria). This function copies them to a table with a maximum number of elements." )
END_SCRIPTDESC();
I can merge the new functions with the changes in your PR myself, so don't worry about this. (And I am aware that GetTouchingEntities()
will need to use ArrayAppend()
)
#define SCRIPT_RW_PATH_ID "MOD" | ||
#define SCRIPT_RW_FULL_PATH_FMT "export/%s" | ||
|
||
class CScriptReadWriteFile : public CAutoGameSystem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Giving VScript the ability to read and write to files was alarming to me at first glance, but now I understand that this exists in later games and is limited to a new directory with a specific size limit per file.
Even still, are there any risks to this system which could be dangerous to users? (including risks in multiplayer)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's not possible to exit to an upper directory, the only issue I can think of is writing very large files to fill up the disk. The same issue exists in L4D2 too. I'm assuming Valve thought about it enough to allow it. I added the already-too-large 64 MB write size because no limit in Valve's vscript seemed odd. I have not tested Dota 2, however, it may or may not have different rules.
For multiplayer it can be used to export data to players (clientside), instead of only saving on server; but the same issue also applies.
Binaries cannot be written or executed; malicious batch/shell scripts can be written, but that still requires the user to manually execute them.
@@ -871,6 +871,11 @@ class IScriptVM | |||
virtual bool ClearValue( HSCRIPT hScope, const char *pszKey ) = 0; | |||
bool ClearValue( const char *pszKey) { return ClearValue( NULL, pszKey ); } | |||
|
|||
#ifdef MAPBASE_VSCRIPT | |||
// virtual void CreateArray(ScriptVariant_t &arr, int size = 0) = 0; | |||
virtual bool ArrayAppend(HSCRIPT hArray, const ScriptVariant_t &val) = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call on this. I was using SetValue()
for my list-filling functions because there's an existing hook in point_template
which uses SetValue()
to create a table of spawned entities. I thought it worked fine based on basic testing, but I apparently didn't test it enough. Thank you.
#define SCRIPT_MAX_FILE_READ_SIZE (16 * 1024) // 16KB | ||
#define SCRIPT_MAX_FILE_WRITE_SIZE (64 * 1024 * 1024) // 64MB | ||
#define SCRIPT_RW_PATH_ID "MOD" | ||
#define SCRIPT_RW_FULL_PATH_FMT "export/%s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Knowing that this is supposed to replace ems/
, I might change this to vscript_io/
to make the folder's origin and purpose more clear to people looking in a mod's root directory.
END_SCRIPTDESC(); | ||
|
||
|
||
class CScriptSaveRestoreUtil : public CAutoGameSystem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has level transition persistence been tested?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Level transition itself wasn't tested, but in singleplayer both client and server IGameSystem::Shutdown()
is not executed until the game process terminates, so they're basically never removed without an explicit removal.
Disconnecting and launching a new map was tested, I don't see why it wouldn't work with transitions particularly.
@@ -4411,7 +4453,11 @@ bool CBaseEntity::AcceptInput( const char *szInputName, CBaseEntity *pActivator, | |||
// found a match | |||
|
|||
// mapper debug message | |||
#ifdef MAPBASE | |||
ConColorMsg( 2, Color(CON_COLOR_DEV_VERBOSE), "(%0.2f) input %s: %s.%s(%s)\n", gpGlobals->curtime, pCaller ? STRING(pCaller->m_iName.Get()) : "<NULL>", GetDebugName(), szInputName, Value.String() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've occasionally received requests/suggestions to color console messages in a similar way to later branches of Source/Source 2. I didn't think it was important enough to do it with the desired consistency, but I hadn't thought about this particular case before. I've had difficulty distinguishing these I/O messages from others in the past, so this might motivate me to finally begin expanding colored console messages to other areas.
I also might change this later to use static Color
instances instead of storing the values in preprocessors.
I don't really have anything else planned to commit to this PR, but it can stay open in case I find an issue (or a proper lookup table for game events). |
I usually merge contribution PRs right before the next update drops, but I want to base the upcoming update's changes off of this and it's still going to be some time before that's ready, so I've decided to go ahead and merge this now. Feel free to open another PR if you need to add more changes. |
vscript additions and fixes
Adding event listeners, entity I/O event modifiers, save/restore utilities, file read/write, misc additions and fixes. Excuse the large commit, they were all written simultaneously over the weeks, it would take too much time to make separate commits.
AcceptInput bypasses the event queue and its inevitable delay in EntFire. FireOutput parameters are set according to how it is in Source 2.
Example usage of event queue functions:
Simulate pausing events:
Events are referenced by their pointers. The validity of the pointer is checked by comparing to the events in the queue.
One thing the users may need to pay attention to is - because of the usage of pointers as IDs - making sure they do not use expired event IDs; because the same addresses can be used by new events.
Changed entity I/O print colour to the colour seen in CSGO (dev level 2) and Source 2 (DeveloperVerbose). Valve probably sets it on the output channels; at the moment my implementation for this is local and messy, using preprocessors at every print.
Increased the squirrel printfunc buffer to the same value as VSquirrel, 2048. Changed vscript print output colour to the colour seen in L4D2 and Source 2, and added Msg as an independent function. Now print, Msg and Warning function (almost*) identical to Source 2.
* In Source 2
Warning
colour is yellow, and vscript error colour is pink.OnSave/OnRestore callbacks allow the mapper to modify the map and logic on game saves and loads. OnRestore also enables users to properly continue entity thinking and event listening.
SaveTable/RestoreTable is a very simple implementation. It basically copies all primitive values from the input table into KeyValues. Memory freed on Shutdown, or with ClearSavedTable.
Example counting:
I don't know why L4D2 does not have a max write size for StringToFile, but I set it to 64 MB. It can easily be removed or changed if it becomes an issue.
I initially chose the output directory as
export/
in "MOD" path. Valve usesems/
(stands for "Expanded Mutation System" from L4D2) in Source 2 (Dota 2) and L4D2. I'm really not sure about this name, you can name it something else.One notable difference of this event listener API from the Source 2 (HLVR) API is the context type. Source 2 uses script tables as contexts, and I'm using strings. In Source 2 the contexts are passed to the function call because of how Lua works. This is unnecessary in Squirrel as the functions can be manually bound to arbitrary 'contexts'.
StopListeningAllToGameEvents("")
stops all events with empty contexts, rather than not doing anything like in Source 2.One known issue is not getting event data on client. Valve probably can easily iterate through the IGameEvent keys in engine, I could only think of manually loading them. All technical details are in the code.
Example game event listening:
Example L4D2 style wrapper:
Usermessages can be used instead of some entities, such as env_fade, env_shake, game_text and env_hudhint. Instead of adding each utility function individually (ScreenShake, ScreenFade, ClientPrint etc) I decided to add an interface to usermessages, which enables so many more possibilities. It only comes with the cost of more function calls from script instead of one. Using messages also doesn't pollute the string pool by allocating each string.
I named it NetMsg and included it in the shared file (although it is server only) to allow future expansions.
Usage:
Multiplayer example:
CEntities::FindByClassnameWithinBox
checks the intersection of the entity world space AABB and the input AABB, while the others check the distance from the entity origin. This function becomes especially useful on entities with large collision boxes.GlobalSys is from Source 2, but with an extra function for getting the whole command line.
Approach and ApproachAngle run faster as SQ functions, compared to calling native C++ functions.
ScriptEntitiesInBox
,ScriptEntitiesAtPoint
,ScriptEntitiesInSphere
used to add entities to table slots of their targetnames, and overwrite previously found entities with the same names. E.g. it would return a single entity in an area with 1000 unnamed entities. I fixed it by changing it to use arrays instead of tables, matching the native UTIL and Valve'sCEntities::FindAll*
functions in VLua.This does "break" compatibility with scripts using these functions; but well, they were already internally broken. I doubt anyone would have been using them.