search

ChiefOfGxBxL / WC3MapTranslator

Translate war3map ⇄ json formats for WarCraft III .w3x maps
https://www.npmjs.com/package/wc3maptranslator
MIT License
73 stars 31 forks source link

Feature/terrain translation #22

Closed ttay24 closed 5 years ago

ttay24 commented 5 years ago

Should take care of #15

ttay24 commented 5 years ago

Do i need to update package.json version for new version so this can be pushed to npm?

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 18

Totals Coverage Status
Change from base Build 12: 3.3%
Covered Lines: 667
Relevant Lines: 722
💛 - Coveralls
ttay24 commented 5 years ago

Also probably need to update wiki for this? Not sure how all the examples work, but probably need to update that as well

ChiefOfGxBxL commented 5 years ago

Hey @ttay24, thanks for working on this! Your changes look good, and it's also a big help to have the warToJson function implemented for terrain.

We'll want to get the wiki updated to reflect these changes. Do you have permission to edit the wiki? If so you're free to do so.

You can also update the version in package.json to 2.0.0 and push that, since this is a major API change (the format in which developers need to specify their terrain has been changed, so is not backwards compatible).

I'll take the translation for a spin this week.

Thanks for the contribution 😃

ttay24 commented 5 years ago

hey, sorry for the delay. I made those changes based on the review if you could take a look whenever you get a chance!

ChiefOfGxBxL commented 5 years ago

No worries -- thanks for continuing to work on this. I will take a look at it this weekend!

ChiefOfGxBxL commented 5 years ago

Hi @ttay24, I've looked over the code and ran tests on my end. So far so good! 👍 The reversion test (which I ran locally using diff) was good to see work because it means you nailed the spec with all those magic values correctly in both translators. In writing previous translators sometimes small mistakes I made caused bits and bytes to be mismatched and were annoying to track down.

From the above list, the last thing I want to check is putting the terrain .w3e file into a WarCraft map to check if the terrain is OK in World Editor. I'll get to this tomorrow when I can grab the attached file on my laptop where I have WarCraft and WE installed.

ttay24 commented 5 years ago

sweet! Let me know how it goes!

Is this thing used in your IceSickle editor? Are there any other translators that have to be written?

ChiefOfGxBxL commented 5 years ago

Works great! The terrain was inserted into a map with Ladik's MPQEditor and you can see the result is good: (ignore the small light rectangle over the terrain, that was just a visual glitch from my screenshot) w3eTranslatorResult

✔️ This is all set to approve! Thank you for all the work you put into this!

Yes, WC3MapTranslator is used in Ice Sickle to generate the entire map, since Ice Sickle stores all the data as JSON both in-memory and when it saves the project to disk. However, since the announcement of WarCraft 3: Reforged, and with Hive WE making great headway, I'm holding off on development of Ice Sickle for now.

What will be interesting to see is what changes are made to the format of war3map files, most likely with terrain (.w3e) and the info file (.w3i). A developer mentioned that when reskinning the terrain for Reforged, cliffs looked off so they're making changes to the underlying terrain system. For the info file, new support for Lua and other changes to the World Editor means the w3i file will have to be updated to support new map settings. Exciting times for WarCraft 3! 😃

ChiefOfGxBxL commented 5 years ago

For your question about translators that still need development, it looks like they're all done. The pathing map and shadow map ones don't exist, but those are fully generated by the terrain and doodad files (like shadow from trees, or pathing is affected by cliffs and blocking doodads).

I'll also publish this PR to NPM later tonight.