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

Incorrect size calculation in RelativeSkeletalPositionSensor #402

Open
insooneelife opened this issue Mar 20, 2020 · 2 comments
Open

Incorrect size calculation in RelativeSkeletalPositionSensor #402

insooneelife opened this issue Mar 20, 2020 · 2 comments
Labels

Comments

@insooneelife
Copy link

insooneelife commented Mar 20, 2020

Hello guys
I found a weird code in RelativeSkeletalPositionSensor.cpp

//code
void URelativeSkeletalPositionSensor::TickSensorComponent(float DeltaTime, ELevelTick TickType, FActorComponentTickFunction* ThisTickFunction) {
	float* FloatBuffer = static_cast<float*>(Buffer);
	for (int i = 0; i < Bones.Num(); i++) {

		FQuat BoneQ = SkeletalMeshComponent->GetBoneQuaternion(Bones[i], EBoneSpaces::WorldSpace);
		FQuat ParentQ = SkeletalMeshComponent->GetBoneQuaternion(ParentBones[i], EBoneSpaces::WorldSpace);
		FQuat Quat = ParentQ.Inverse() * BoneQ;
		FloatBuffer[4 * i] = Quat.X;
		FloatBuffer[4 * i + 1] = Quat.Y;
		FloatBuffer[4 * i + 2] = Quat.Z;
		FloatBuffer[4 * i + 3] = Quat.W;
	}
}

int URelativeSkeletalPositionSensor::GetNumItems() {
	return this->Bones.Num();
}

at the code, TickSensorComponent()
~
FloatBuffer[4 * i] = Quat.X;
FloatBuffer[4 * i + 1] = Quat.Y;
FloatBuffer[4 * i + 2] = Quat.Z;
FloatBuffer[4 * i + 3] = Quat.W;
~
they are accessing array index up to BoneNum*4,

but in the code, GetNumItems()
it just returns Bone.Num().

Shouldn't this code be changed like this?

int URelativeSkeletalPositionSensor::GetNumItems() {
	return this->Bones.Num() * 4;
}
@insooneelife insooneelife changed the title Abount holodeck-engine RelativeSkeletalPositionSensor About holodeck-engine RelativeSkeletalPositionSensor Mar 24, 2020
@jaydenmilne jaydenmilne added this to the 0.4.0 milestone Mar 31, 2020
@jaydenmilne
Copy link
Contributor

Yes, that probably should be changed, but it currently isn't causing any issues. That GetNumItems is used to allocate the size of the shared memory buffer if the client hasn't already done so. Since the client normally runs first, and it's calculation is correct, we haven't seen anything.

Thank you for pointing this out.

We should spot check the rest of the sensors to make sure that they are the same on both sides.

@jaydenmilne jaydenmilne changed the title About holodeck-engine RelativeSkeletalPositionSensor Incorrect size calculation in RelativeSkeletalPositionSensor Mar 31, 2020
@daniekpo
Copy link
Contributor

daniekpo commented May 6, 2020

@jaydenmilne, from your comment above, is this a bug?

@MaxRobinsonTheGreat MaxRobinsonTheGreat removed this from the 0.3.2 milestone Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants