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

VScript changes #47

Merged
merged 7 commits into from
Sep 23, 2020
Merged

VScript changes #47

merged 7 commits into from
Sep 23, 2020

Conversation

samisalreadytaken
Copy link

VScript changes that reflect additional utility and consistency with Source 2, L4D2 and CSGO.

CBaseEntity::SetThinkFunction sets the "ScriptThink" context (script func defined in entity thinkfunction kv), CBaseEntity::SetThink sets the "ScriptThinkH" context. SetThink stores and executes the handle directly, thus allows closure inputs.

On this matter, I suggest doing what Source 2 does, and remove the 'thinkfunction' keyvalue and the CBaseEntity::ScriptThink function (by extension SetThinkFunction too), and let the user handle their think functions more freely. Having it as a keyvalue forces a recompile if the function name is to change; but using the new SetThink function, they can just do it all in the script file self.SetThink( function(){}, 0 )

CBaseEntity::SetContextThink is also a possible addition for the future.

CBaseEntity::SetThinkFunction includes initial delay and removes the need for AddThinkToEnt. I suggest removal of the global AddThinkToEnt.

CDebugOverlayScriptHelper resolves #29. The reason NDebugOverlay code is copied here instead of calling it is because of the player and LOS checks. Rather than to change the functionality of the original code, I copied the important parts.

There is no need for metamethod stubs (excluding _tostring and IsValid). They will fail in all conditions except when the behaviour is manually defined, then there would already be no need for stubs.

The incorrect GetLeftVector naming was fixed, and GetPlayerUserId in L4D2 was also changed to its native name GetUserID in Source 2.

I'm not sure what the purpose of SetRenderColorR,G,B are (they're not even used in game code), they seem unnecessary when there is SetRenderColor with R,G,B params, also as seen in Source 2.

There is a SetOwner duplicate in baseentity (in Valve's code): ScriptSetOwner and (virtual) SetScriptOwnerEntity. Neither have any references other than script declarations. They could be cleaned up.

c_baseentity.h
c_baseentity.cpp
   - Renamed GetLeftVector to GetRightVector
   - Renamed GetTeamNumber to GetTeam

baseentity.h
baseentity.cpp
   - Renamed GetLeftVector to GetRightVector
   - Renamed Get/SetRender functions (GetAlpha -> GetRenderAlpha)
   - Fixed CBaseEntity::GetScriptId
   - Added hook CBaseEntity::UpdateOnRemove
   - Added CBaseEntity::GetOrCreatePrivateScriptScope
   - Added CBaseEntity::GetDebugName
   - Added CBaseEntity::GetGravity
   - Added CBaseEntity::SetGravity
   - Added CBaseEntity::GetFriction
   - Added CBaseEntity::SetFriction
   - Added CBaseEntity::GetMass
   - Added CBaseEntity::SetMass
   - Added CBaseEntity::SetName (SetNameAsCStr)
   - Added CBaseEntity::SetParent (ScriptSetParent)
   - Added CBaseEntity::SetThink (ScriptSetThink)
   - Added CBaseEntity::StopThink (ScriptStopThink)
   - Added CBaseEntity::SetThinkFunction (ScriptSetThinkFunction)
   - Added CBaseEntity::StopThinkFunction (ScriptStopThinkFunction)
   - Added CBaseEntity::ApplyAbsVelocityImpulse
   - Added CBaseEntity::ApplyLocalAngularVelocityImpulse

player.h
player.cpp
   - Renamed GetPlayerUserId to GetUserID
   - Added CBasePlayer::GetFOV
   - Added CBasePlayer::GetFOVOwner (ScriptGetFOVOwner)
   - Added CBasePlayer::SetFOV (ScriptSetFOV)

vscript_consts_shared.cpp
   -Added RAD2DEG, DEG2RAD, MAX_COORD_FLOAT, MAX_TRACE_LENGTH

vscript_funcs_shared.cpp
   - Renamed IsClientScript,IsServerScript to IsClient,IsServer
   - Added IsDedicatedServer
   - Added NPrint (Con_NPrintf)
   - Removed DebugDrawBoxDirection (debugoverlay.BoxDirection)
   - Removed DebugDrawText (debugoverlay.Text)

vscript_server.cpp
   - Added CDebugOverlayScriptHelper (debugoverlay)
   - Added CEntities::GetLocalPlayer
   - Added DispatchParticleEffect (ScriptDispatchParticleEffect)
   - Removed DebugDrawBox (debugoverlay.Box)
   - Removed DebugDrawLine (debugoverlay.Line)

vscript_squirrel.cpp
   - Changed stub error messages for consistency and clarity
   - Changed errorfunc output to Warning instead of Msg
   - Added Msg, Warning
   - Added clamp, min, max, RemapVal, RemapValClamped
   - Added sqstdtime.h
   - Added clock, time, date
@Blixibon
Copy link
Member

I love these changes and I would normally really want to merge this PR, but it currently breaks too much existing functionality to be acceptable into Mapbase as-is.

  • Consistency with other Source (2) games is normally welcome, but I know some scripts are already using the functions renamed in this PR and it may be too late to change them for this. It's possible that all of the existing scripts are in mods with their own versions of Mapbase and if the involved mods were updated to a version of Mapbase which uses these new names, those scripts could presumably be updated as well. This may require further discussion.

  • The metamethods removed in this PR are supposed to allow programmers to add member variables and calculations for script classes, like Quaternion. I don't really understand the reasoning for why they were removed since they're supposed to do nothing by default; Script classes are supposed to override them when they need to use them. How would there be no need for stubs if they're manually defined?

Maintaining legacy support is something I neglected to mention in the contribution guidelines, so I'll add a line for that in the next update.

Unless we decide to keep these renames in, I strongly recommend adding another commit which restores GetPlayerUserID, GetLeftVector, GetTeamNumber, etc. as deprecated names and also restores the removed metamethods. I could do this myself as well if you don't have time.

(I will, however, unconditionally accept turning IsServerScript() and IsClientScript() into their shortened versions because I know nobody would've been using them yet)


I'm not sure what the purpose of SetRenderColorR,G,B are (they're not even used in game code), they seem unnecessary when there is SetRenderColor with R,G,B params, also as seen in Source 2.

They're shortcuts for when you want to set one color channel, but not the other and you don't want to type out something like self.SetRenderColor( 255, self.GetRenderColorG(), self.GetRenderColorB() ) or involve a separate vector.

@Blixibon Blixibon added Enhancement New feature or request VScript Mapbase - Involves VScript labels Aug 30, 2020
@samisalreadytaken
Copy link
Author

samisalreadytaken commented Aug 30, 2020

After testing the metamethods a bit more, I see you have a different design than what I had in my mind; reverted.

You can add back the renamings as you see fit (Done). The wiki could also only include the new names to discourage deprecated name usage.

@Blixibon Blixibon merged commit 5c20518 into mapbase-source:master Sep 23, 2020
VDeltaGabriel pushed a commit to Project-P2009/p2009_sdk that referenced this pull request Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request VScript Mapbase - Involves VScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VScript debug draw-related functions not functional
3 participants