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

Added 7.29 map and minimap assets. #48

Merged
merged 1 commit into from
Jul 1, 2021

Conversation

likwidoxigen
Copy link
Contributor

Nothing fancy here just added the 7.29 assets.
I figure it makes sense to switch to the current patch maps and then back-port older-maps with the pick by date.
If you'd rather not go that route just decline the request and then I'll remove it.

@timkurvers
Copy link
Owner

Awesome! That sounds like a good approach.

The 7.23 images originated from the Dota 2 wiki on Gamepedia. Is that the case for these files too?

@likwidoxigen
Copy link
Contributor Author

likwidoxigen commented Jun 30, 2021

These files I generated. Devilesk (the original person who would generate the map) left some great instructions for the process. I took the old map and lined up fountains (to make sure structures placed correctly) and then double checked their placement with a current replay.
As far as the minimap I did custom scalling on the full size map over the existing mini-map to make sure everything lined up. It isn't specifically the mini-map asset from Dota2, just a scaled full size map, but it works the same. I'm still figuring out how to find/extract the real minimap.

@timkurvers
Copy link
Owner

Ah excellent! If you like we can add a note to the README that you are the creator of these assets specifically.

One minor thing: it seems Husky did not enforce commit messages to adhere to the guidelines for you, would you like to change your commit message? Alternatively, I might be able to do that in the merge/squash process.

@likwidoxigen
Copy link
Contributor Author

Sure thing, that would be great :)

Sorry about that. Looking now I see that I don't have the HUSKY_GIT_PARAMS var defined anywhere. Is there a missing .env sample file? After we sort this out I'd be happy to work up a contributors guide as well.

@timkurvers
Copy link
Owner

timkurvers commented Jul 1, 2021

Sorry about that. Looking now I see that I don't have the HUSKY_GIT_PARAMS var defined anywhere. Is there a missing .env sample file? After we sort this out I'd be happy to work up a contributors guide as well.

These git hooks should in theory have installed themselves automatically on npm install as part of husky.

At least they do here on my end:

$ npm install
...
> husky@4.3.0 install /redota/node_modules/husky
> node husky install
husky > Setting up git hooks
husky > Done

Edit: Just to double check, it's Husky 4 being installed on your end? Husky 7 seems to have changed the way commands/hooks are defined.

@likwidoxigen
Copy link
Contributor Author

Ahh ok, that was my bad. Needed to have git installed on the docker image. It did take me far too long to think of looking at a prior commit for how to structure the message but it's installed and verified working.
Re-wording could probably be done at merge? At this point I'd probably have to wipe the branch and submit a new pull request. Let me know if you want me to wipe and re-submit.

Thanks!

@timkurvers
Copy link
Owner

Ah, that makes sense! No worries, I'll have a look.

@timkurvers timkurvers merged commit 0ddd5d8 into timkurvers:master Jul 1, 2021
@timkurvers
Copy link
Owner

Thanks! If you would like to be credited in the README in a certain way, feel free to open a PR for that. Otherwise I'll do that a bit later this week.

@timkurvers
Copy link
Owner

Added the resource note to the README in 051749c.

timkurvers pushed a commit that referenced this pull request Nov 2, 2021
Co-authored-by: Bryce Leo <bryce.leo@trainingvault.us>
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