Skip to content

add show_player_in_hltv #120

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

Merged
merged 5 commits into from
Mar 16, 2025
Merged

Conversation

khanghugo
Copy link
Contributor

closes #117

The implementation is very similar to BXT. I also realize that I have been taking this feature for granted, recording a few demos with this feature without knowing it is a feature.

There is Cmd_FindCmd but I just feel like it is easier to just implement my own.

let cmd_function = &*curr_cmd_function;
let curr_name = CStr::from_ptr(cmd_function.name as *mut i8);

if curr_name.to_str().ok()? == name {
Copy link
Owner

Choose a reason for hiding this comment

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

This shouldn't early-return upon to_str() failure, it just means we need to skip this cmd.

Suggested change
if curr_name.to_str().ok()? == name {
if curr_name.to_str() == Some(name) {

&& engine::ClientDLL_IsThirdPerson.is_set(marker)
&& engine::CL_IsSpectateOnly.is_set(marker)
&& engine::pmove.is_set(marker)
&& unsafe { engine::find_cmd(marker, "dem_forcehltv") }.is_some()
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, I don't like this logic involving some engine state in is_enabled(). Perhaps should find this somewhere similar to how we find pointers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These HLTV and demo codes are inside demoplayer.so (or at the very least, not inside engine code). I don't see any references to static HLTV variables inside engine. The reason why there is cvar check in the first place is because BXT does it. I suspect it is to avoid crashing or unintended behaviors. @SmileyAG may know why it is needed.

Anyways, if it doesn't look like what you want, it is possible to store a static variable caching the result of this find_cmd during run time to check that dem_forcehltv actually exists.


This is the fix for that.

Alternatively, with this fix, it is possible to use `playdemo` with `thirdperson` to have the player model show up to just record. \
Copy link
Owner

Choose a reason for hiding this comment

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

Does this mean the dem_forcehltv 1 check is not needed?

Copy link
Contributor Author

@khanghugo khanghugo Mar 14, 2025

Choose a reason for hiding this comment

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

No you don't need that check. dem_forcehltv 1 is for viewdemo. When you do playdemo, forcing HLTV doesn't do anything.

Copy link
Owner

Choose a reason for hiding this comment

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

Then maybe remove the whole dem_forcehltv check and find_cmd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I guess I didn't explain that clearly.

To view the player model when playing back a demo, there are two ways. The first way is doing playdemo and thirdperson. You don't have free camera movement with that. The second way is viewdemo with dem_forcehltv 1. With that, you have free camera movement.

The playdemo route is for straightforward loading a campath and then recording. The viewdemo route is for looking around and seeing what you might want to do.

I think it is possible to just remove the dem_forcehltv and then find_cmd. That wouldn't reduce any functionalities. It is just that the original BXT implementation has this check so I just have it there to make sure things are on parity. And again, tagging @SmileyAG here and hope he can explain why he has it in his implementation.

@khanghugo
Copy link
Contributor Author

After some more testing on this, it seems like playdemo and then thirdperson displays the player model without the fix. viewdemo and dem_forcehltv 1 route does need this fix.

The reason why this PR/fix exists is so that people can record a demo in third person. Personally, after learning that playdemo and thirdperson works just fine, I would just stick with that. However, some people may prefer the HLTV way and this fix is for them.

@YaLTeR
Copy link
Owner

YaLTeR commented Mar 16, 2025

If I wrote ok, then commit via github UI and let's merge

khanghugo and others added 2 commits March 16, 2025 09:10
@khanghugo
Copy link
Contributor Author

Let's do it.

@YaLTeR YaLTeR merged commit 608fa9f into YaLTeR:master Mar 16, 2025
13 checks passed
@YaLTeR
Copy link
Owner

YaLTeR commented Mar 16, 2025

Thanks

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.

Draw third person model in dem_forcehltv freecam
2 participants