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 vgui #169

Merged
merged 1 commit into from
Oct 26, 2022
Merged

Conversation

samisalreadytaken
Copy link

@samisalreadytaken samisalreadytaken commented Dec 22, 2021

This implementation is not final, but it is stable.

Example code and videos:

https://github.com/samisalreadytaken/vscripts/tree/master/mapbase
https://gist.github.com/samisalreadytaken/4847ede81a7222b66dc82eac8622904e

https://youtu.be/PrND1JsXduo
https://youtu.be/I4Y_kLR2CQ0
https://youtu.be/Oh5JpBHXWYM

Some notes:

  • Most #if 0'd blocks are either untested, or usefulness of them are uncertain.

  • Should the function to set overrides/callbacks be named SetCallback(), or something else? Panorama uses SetPanelEvent() for registering callbacks.

  • A future change to add is integration with C_VGuiScreen entity to display script panels. This would also be a simpler alternative to manual 3D world painting.

  • A TODO is a convenient method to control the visibility of existing HUD elements. The current hack is to use the hidehud convar which is manually reset on level shutdown. (See hud_override.nut)


PR Checklist

  • My PR follows all guidelines in the CONTRIBUTING.md file
  • My PR targets a develop branch OR targets another branch with a specific goal in mind

Copy link

@z33ky z33ky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are minor fomatting issues with inconsistent spacing around parenthesis in control-flow statements and function parameters. It's not really concerning, just pointing it out.
Missing spaces after SCRIPT_SINGLETON (BEGIN_SCRIPTDESC_ROOT_NAMED()) also caught my eye.

The name of _panel diverges from the more common style (m_pPanel), though it has precedent in the original VGUI code.
This and m_hScriptInstance have accessors and could become private; I think only ResolveChildren() would need to be modified, either by being friended or by becoming a static method of IScriptVGUIObject. Again, just pointing it out.

There are a couple of loops to search for matching panels in g_ScriptPanels. That could be refactored to a function.

There are two unrelated changes in sp/src/game/client/vscript_client.cpp which should get their own dedicated commit (replacement of loop with V_memset() and removal of a call to g_pScriptVM->ClearValue( m_ScriptScope, "entity" ) in CScriptMaterialProxy::OnBind()).

There are a couple of places where references to a static Vector is returned.
I'm not sure how the script FFI deals with these - if a script were to call it twice, would both variables in the script also refer to the same object? I.e. would something like this fail:

local top = ScreenToWorld(0.5, 0);
local bottom = ScreenToWorld(0.5, 1);
assert( top != bottom );

sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
@samisalreadytaken
Copy link
Author

_vpanel and m_hScriptInstance are also accessed from derived helper classes and from CScriptVGUI::Create(). There could be setter functions, but I don't think it really matters.

Returned Vectors to script functions are reallocated in the VM, returning static Vector is Valve's convention. Though testing local Vector return, it seems to work fine; perhaps it's a difference in the implementation, or I'm forgetting something.

I will push some changes later.

sp/src/game/client/mapbase/vscript_vgui.cpp Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
@samisalreadytaken samisalreadytaken marked this pull request as draft January 7, 2022 22:17
@samisalreadytaken samisalreadytaken force-pushed the vscript_vgui branch 3 times, most recently from 4b4b0d3 to a6dba10 Compare January 15, 2022 15:57
@samisalreadytaken samisalreadytaken force-pushed the vscript_vgui branch 2 times, most recently from 1a8806e to dd8314f Compare January 29, 2022 19:12
@samisalreadytaken samisalreadytaken marked this pull request as ready for review January 29, 2022 19:27
@samisalreadytaken samisalreadytaken marked this pull request as draft January 31, 2022 18:25
@samisalreadytaken samisalreadytaken force-pushed the vscript_vgui branch 3 times, most recently from 09065cb to 368796a Compare February 4, 2022 15:23
@samisalreadytaken samisalreadytaken marked this pull request as ready for review February 4, 2022 15:27
@samisalreadytaken samisalreadytaken force-pushed the vscript_vgui branch 3 times, most recently from 68c63d0 to 36dd406 Compare February 8, 2022 19:02
@samisalreadytaken samisalreadytaken marked this pull request as draft February 11, 2022 17:58
@samisalreadytaken samisalreadytaken marked this pull request as ready for review February 13, 2022 16:47
@samisalreadytaken samisalreadytaken force-pushed the vscript_vgui branch 3 times, most recently from bdd338f to 7cceb59 Compare February 18, 2022 23:46
Copy link
Member

@Blixibon Blixibon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's not any practical way for me to test everything this has, although I did try using the HUD replacement scripts from your repo and messed around with the save icon script you provided. I experienced an Overflowed reading usercmd data (check sending and receiving code for mismatches)! error from the fake Steam client, which I'm assuming isn't related to the VScript VGUI code itself, but I'm otherwise in agreement that this seems stable. Before I approve this, I'm requesting a change I had to make for it to compile on my machine.

I'm really blown away by the capabilities of this implementation. This has the potential to open custom VGUI creation to a much wider audience.


Should the function to set overrides/callbacks be named SetCallback(), or something else? Panorama uses SetPanelEvent() for registering callbacks.

The context of SetCallback is specific enough that I think the name works best as-is than it would as something more explicit.

sp/src/game/client/mapbase/vscript_vgui.cpp Outdated Show resolved Hide resolved
@samisalreadytaken
Copy link
Author

The Steam notifications & achievements code works fine, are you sure you've included it on both server and client, and there are no errors on server? That error happens when client sends a message and server doesn't receive it. I also updated that code for save/restore few days ago (and rebased because they're code for an unreleased WIP feature so I don't care about history), although unlikely, you might have used a broken version.

Additionally I removed WorldToScreen() for now because I'm not sure about its output parameter. I had initially chosen 0,1 indices for an array because arrays are the lightest containers in Squirrel, but it could use x,y as well. The HSCRIPT parameter memory leak is also unideal, people shouldn't really use it.

Not too worried about temporarily removing this because it's already possible to manually calculate this.

@Blixibon
Copy link
Member

Ah, I didn't realize it was meant to run on both the server and the client. I saw that the save icon script was client only and I didn't realize your other HUD scripts were meant to be shared. I'm surprised I missed that, but sorry for the confusion.

@samisalreadytaken
Copy link
Author

The Steam scripts are only shared for the achievement manager (serverside control and verification), and the HUD scripts are shared to be able to get suit information because NetProps doesn't work.

@Blixibon Blixibon merged commit 63a8882 into mapbase-source:develop Oct 26, 2022
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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants