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

Feature: Animation changes #95

Merged
merged 5 commits into from
Jun 11, 2016

Conversation

fallenoak
Copy link
Member

three's new animation system makes it quite a bit easier to work with the data present in M2 animations. This PR brings Blizzardry more in line with what three expects, and produces somewhat leaner output-- particularly when serializing and deserializing.

Recent versions of three use an animation system that fits better
with how WoW stores animation data. It's still helpful to split
animation tracks apart, but it's no longer necessary to create
keyframe objects for each individual keyframe.
Newer versions of three can work directly with arrays of values,
including for complex types like vectors and quaternions. Since
parsing dense content like animations into full-blown objects
is rather overkill, we can now start to move away from that
approach.
@@ -0,0 +1,15 @@
import r from 'restructure';

class Color16 {
Copy link
Member Author

Choose a reason for hiding this comment

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

It'd probably be okay to call this something like SignedFixed16 and read from the stream as an int16le rather than uint16le, but I find it weird to do that for values that can't be negative (like transparency and color values).

Hence... Color16 -- your one-stop shop for values between 0.0 and 1.0 stored as shorts with values between 0 and 32767.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@fallenoak
Copy link
Member Author

This PR will break Wowser until wowserhq/wowser#145 is merged.

@@ -0,0 +1,3 @@
import r from 'restructure';

export default new r.Array(r.floatle, 3);
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this the same as float3? I'd be totally in favour of normalizing all of these to make things more clear 👍

Copy link
Member Author

@fallenoak fallenoak Jun 9, 2016

Choose a reason for hiding this comment

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

My thought was that we should have a consistent naming scheme, and float3 would conflict with things like float16 (for a single fixed-point 16-bit float).

Since float3 is a vector made up of 32-bit floats, I thought maybe we should move toward vec3float32 as a more consistent name.

If you're cool with that, I can search and replace all float3s to vec3float32s.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, sounds perfect 👍

Copy link
Member

Choose a reason for hiding this comment

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

How would we differentiate between an array of three float values – currently float3, vec3float32 – and an object with x, y, z properties – currently Vec3Float?

Copy link
Member Author

@fallenoak fallenoak Jun 10, 2016

Choose a reason for hiding this comment

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

Recap from Gitter: array types now use <type>array<len> as a naming convention, like so:
float32array3 -> [1, 2, 3]

This should resolve naming confusion with types that involve named properties.

* Removed all uses of float2 and float3

* Renamed array vector types to <type>array<len> convention
@timkurvers
Copy link
Member

🎉 Awesome!

@timkurvers timkurvers merged commit 1d14349 into wowserhq:master Jun 11, 2016
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