Skip to content

Characters now render correctly even if their scale is incredibly large or incredibly small. - #144

Open
Glimps3 wants to merge 7 commits into
WolfireGames:mainfrom
Glimps3:character-scale-related-fixes-having-to-do-with-rendering
Open

Glimps3 wants to merge 7 commits into
WolfireGames:mainfrom
Glimps3:character-scale-related-fixes-having-to-do-with-rendering

Conversation

@Glimps3

@Glimps3 Glimps3 commented Sep 7, 2024

Copy link
Copy Markdown

There's a textfile that explains stuffs.

Comment thread Source/Objects/movementobject.cpp Outdated
}

const float kCullRadius = 2.0f;
float kCullRadius = 2.0f;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a global variable, so if it's changed inside one object, it's changed for every single object being drawn. I don't think that's the right solution. You could copy the constant's value to a local variable inside the function(s) it's used in, or you could make it an member variable of the movement object class, so that it is instanced per object.

Comment thread Source/Objects/movementobject.cpp Outdated
if (GetScriptParams()->HasParam("Character Scale")) {
rigged_object_->SetCharScale(GetScriptParams()->ASGetFloat("Character Scale"));

float new_frust = GetScriptParams()->ASGetFloat("Character Scale") * 2;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This ASGetFloat call is already being done two lines above. Ideally we'd get rid of all these per-frame calls into the angelscript context entirely because this sort of variable lookup is relatively slow (compared to using the value of a variable defined and used from C++). But since you didn't put that code there, you definitely don't have to fix it. But ideally you also should avoid multiplying it.

For now, I think it'd be best to put the result of ASGetFloat in a local variable and reuse the value here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh, I expanded the code above. For some reason I assumed this was in draw, due to "frust" being the name of the variable. This is actually in CreateRiggedObject, so it's less of a concern.

Still, generally a good practice to avoid copying code that calls into chains of accessors. Just do it once in the function, assign it to a local variable, and reuse the result.

Comment thread Source/Objects/movementobject.cpp Outdated
kCullRadius = 200.0f;
} else {
kCullRadius = new_frust;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This code is basically kCullRadius = clamp(new_frust, 2.0f, 200.0f);

Not sure if we have such a clamp implementation in our codebase already, but if we do, this would be a perfect place to use it.

Comment thread Source/Objects/riggedobject.cpp Outdated
char_scale = val * character_script_getter->GetDefaultScale();
model_char_scale = char_scale / character_script_getter->GetModelScale();
float default_scale = character_script_getter->GetDefaultScale();
char_scale = val * default_scale <= 0.01f ? 0.01f : val * default_scale;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

like clamp above, this would be a good use for a max function


I am gonna note that this really does only (hopefully) fix rendering stuff, very large or small characters are still wonk in most other gameplay ways (ragdolling/ledge-climbing/fighting and such) though that makes sense considering Overgrowth never was built around that.
Well... Except for rats having bad ledge-climb stuff, dog attacks sometimes hitting even if their fist/foot isn't close to the player, I think cats and wolves suffer from this issue too.
I think that the hitbox issue can be solved via the "range_adjust" thingy I put up a while back. (This _does_ kinda come with the issue of it being cumbersome to have copies of each different attack for each different race just so they have fitting range_adjust values, but personally I think it's worth it for the sake of fairness/balance since getting hit by phantom dog paws isn't the most fun.) No newline at end of file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be checked in, unless it's in the commit message or comments/description of the PR.

If the code itself is confusing, there may be value to putting some of this in comments directly with the code it applies to.

@Glimps3

Glimps3 commented Sep 10, 2024

Copy link
Copy Markdown
Author

@Glimps3

Glimps3 commented Sep 12, 2024

Copy link
Copy Markdown
Author

Updates:
kCullRadius has now been made a member variable of the movement object class.
'clamp' and 'max" are now used where appropriate.
There's that thing about avoiding multiple accessor-chain calls which is now in place, and I also went back through the "Disable Editor Tools In Media Mode" thing and did the same there too.

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.

2 participants