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

Add the Elvish Hunter line to mainline #7329

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

CelticMinstrel
Copy link
Member

These are sort of a relic from the pre-Quenoth desert elves, only converted back to a wood elf. I think it's a nice flavour unit to have for some campaigns.

@github-actions github-actions bot added Graphics Issues that involve the graphics engine or assets. Units Issues that involve unit definitions or their implementation in the engine. labels Jan 25, 2023
@CelticMinstrel CelticMinstrel marked this pull request as draft January 25, 2023 19:00
@CelticMinstrel
Copy link
Member Author

I converted it to draft because I noticed the unit files still need updating.

@github-actions github-actions bot added the UI User interface issues, including both back-end and front-end issues. label Jan 26, 2023
@CelticMinstrel CelticMinstrel marked this pull request as ready for review January 26, 2023 04:44
Copy link
Member

@knyghtmare knyghtmare left a comment

Choose a reason for hiding this comment

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

Seems okay, apart from that cfg file making into the images/units/elves-wood directory

data/core/images/units/elves-wood/prowler.cfg Outdated Show resolved Hide resolved
src/achievements.hpp Outdated Show resolved Hide resolved
src/gui/dialogs/achievements_dialog.cpp Outdated Show resolved Hide resolved
@CelticMinstrel CelticMinstrel removed the UI User interface issues, including both back-end and front-end issues. label Jan 26, 2023
@doofus-01
Copy link
Member

I've got no issue with the concept and do understand the motivation, but these are old frankensprites. I'd need to see a little more effort before I can approve.

Whatever way this gets resolved will be tied to all the other things currently being proposed.

@cooljeanius
Copy link
Contributor

cooljeanius commented Jan 31, 2023

I hope @Elvish-Hunter doesn't mind my requesting a review from him just based on his name... 😃

Copy link
Contributor

@stevecotton stevecotton left a comment

Choose a reason for hiding this comment

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

I've got no issue with the concept and do understand the motivation, but these are old frankensprites. I'd need to see a little more effort before I can approve.

Marking this as "changes requested" because otherwise it shows up in the PR list as "approved". When @doofus-01's comment is addressed, please feel free to remove me from the reviewers list.

hills=40
sand=60
[/defense]
description=_"Elvish hunters are specialized in trapping unwary opponents from a safe distance to ease the task of the swordsmen."
Copy link
Contributor

Choose a reason for hiding this comment

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

For a mainline unit, I'd expect a longer text. Maybe something about not all groups having shamans, or about the trappers having more movement than the shamans, maybe that the hunters aren't too bad in melee.

Probably should be something more gender-neutral than "swordsmen".

Copy link
Member Author

@CelticMinstrel CelticMinstrel Jan 31, 2023

Choose a reason for hiding this comment

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

I seem to be hitting a block on how to make it longer. Your suggestions make sense, but turning that into reasonable prose is harder than it sounds.

I'm not opposed to making it gender-neutral, but it's probably mainly referring to the fighter line which is mechanically all-male.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this is still a problem, I can write one. You'd just have to tell me what it needs to say. (If the unit is even going to be added)

@Jonathan-Kelly
Copy link
Contributor

Honestly, this old unit seems quite redundant with the elvish fighter and elvish shaman. So the effort it would take to update its mediocre looking sprite would probably be better applied to more useful unit designs.

Copy link
Member

@doofus-01 doofus-01 left a comment

Choose a reason for hiding this comment

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

Needs a portrait and sprites would need to be updated/animated.

@stevecotton stevecotton dismissed their stale review February 13, 2023 03:48

Dismissed temporary review that was added because doofus-01 requested changes but didn't at that time use Github's "request changes" status.

@stevecotton
Copy link
Contributor

To make the unit more interesting, maybe "their skill at spotting routes to follow prey lets them give hints to adjacent units about the fastest path", and give them tailwind+1.

@Hejnewar
Copy link
Contributor

To make the unit more interesting, maybe "their skill at spotting routes to follow prey lets them give hints to adjacent units about the fastest path", and give them tailwind+1.

Believe me you dont want to make them even more op then they already are. This unit would end up at like 28g.

@cooljeanius
Copy link
Contributor

this PR should be updated so that it passes the new copyright checker CI task

@cooljeanius
Copy link
Contributor

this PR should be updated so that it passes the new copyright checker CI task

@CelticMinstrel could you please look into this?

@CelticMinstrel
Copy link
Member Author

I don't know where the Elvish Hunter unit originated. As I recall, I got these sprites from either IftU or AtS, but I do not know where Iris obtained them before that. There used to be a hunter in UtBS, but that was a "brown" version of this one. I'm not sure exactly what relationship the UtBS sprites have with these ones; either one could've been based on the other.

Given that it's pretty close to the archer, I'd guess it's probably GPL, but I don't know for sure.

@doofus-01
Copy link
Member

The images would need some changes/additions anyways, so no need to worry about the CSV for now.

@cooljeanius
Copy link
Contributor

I don't know where the Elvish Hunter unit originated. As I recall, I got these sprites from either IftU or AtS, but I do not know where Iris obtained them before that.

I think she drew them herself:
https://forums.wesnoth.org/viewtopic.php?p=255787&hilit=Elvish+Hunter#p255787
https://forums.wesnoth.org/viewtopic.php?p=288729&hilit=Elvish+Hunter#p288729
https://forums.wesnoth.org/viewtopic.php?p=290636&hilit=Elvish+Hunter#p290636
@irydacea can you confirm?

@knyghtmare
Copy link
Member

One thing I agree with doofus here is that the level 2 and level 3 need the minimal animations done. They are just the base image for now. However, given that the current Sylph is also just a base image, I am not sure if this minimal requirement is still in effect.

@irydacea
Copy link
Member

irydacea commented Aug 7, 2024

@cooljeanius Not entirely true. Quoting myself from the first post you linked:

These are basically rusty frankensteinings of existing art, more spec. Elvish Fighter and Archer.

Thus, technically they are just edits of Jetrel's (IIRC) version of both units.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graphics Issues that involve the graphics engine or assets. Units Issues that involve unit definitions or their implementation in the engine.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants