search

twolfson / layout

Organize and layout items based on various algorithms
MIT License
113 stars 15 forks source link

Switched btree dep to boxpack #7

Closed jonathanrdelgado closed 9 years ago

jonathanrdelgado commented 9 years ago

Fixes #6

Great suggestion on boxpack, really easy switch.

All of the iterations for the input and output properties are no longer necessary, the new module doesn't use the naming shortcuts the old one did.

Tests are passing. I also regenerated a handful of my project spritesheets, all were identical through the switch, but please double check before merging, you probably know of a few edge cases?

twolfson commented 9 years ago

Everything looks great at a glance. I will review more in depth tonight. Thanks for the fast turnaround!

twolfson commented 9 years ago

It looks like we are dropping metadata but I should have maintained that in the test suite =/

Going to take a look at the boxpack source. Worst case: We add metadata back after the algorithm runs.

jonathanrdelgado commented 9 years ago

@twolfson Good catch, didn't notice that.

Any suggestion on doing so? I assume the array order may not be persistent?

twolfson commented 9 years ago

Ah, it looks like it is persisted but hidden via Object.create:

https://github.com/munro/boxpack/blob/3809aa551a7603f2fee7496fcdaabc4c270d8ddd/boxpack.js#L71-L75

When I log the meta property directly, it appears. I am going to tack on another commit to add tests and re-expose the meta key as an enumerable property.

Thanks again!

jonathanrdelgado commented 9 years ago

@twolfson Phenomenal. Thank you for your quick replies!

twolfson commented 9 years ago

Thanks for your quick replies as well =)

This has been merged and released in 2.1.0. I will make updates/releases in the spritesmith family shortly.

twolfson commented 9 years ago

Alright, the family has been upgraded:

jonathanrdelgado commented 9 years ago

Awesome, thanks again! I really appreciate it.