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

Move the Naga Hunter unit to core #7326

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

Move the Naga Hunter unit to core #7326

wants to merge 3 commits into from

Conversation

CelticMinstrel
Copy link
Member

This allows addons to safely use it without having to copy the files into their own addon directory.

@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
@stevecotton
Copy link
Contributor

Please add a log to the changelog_entries directory.

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.

PR seems good to go, but I think there was animations for the Naga Assassin unit. Might add in a follow-up PR

@Pentarctagon
Copy link
Member

Pentarctagon commented Jan 26, 2023

Needs doofus-01's review for the new Naga Assassin sprite.

@Jonathan-Kelly
Copy link
Contributor

This old unit heavily overlaps with the new naga dirkfang line, specifically the ophidian branch. Why not just give the ophidian or sicarius poison on its ranged attack and then discontinue this unit altogether? Replace all instances of it in UtBS with the new dirkfang line?

@doofus-01
Copy link
Member

This unit is fine as a campaign-specific unit, but I'm opposed to moving it to core. As Jonathan-Kelley says, there is overlap with the dirkfang line, and I remember seeing a more creative version of Naga Assassin in UMC that this might conflict with. This has a Lordbob (I think?) portrait, but otherwise there's nothing special here.

Like I'd mentioned in #7335 - if there is some middle ground to directly mainlining, that makes the assets available to all without committing this to core, I would not object.

@sevu sevu closed this May 6, 2023
@CelticMinstrel
Copy link
Member Author

Why was this closed?

@CelticMinstrel CelticMinstrel reopened this May 6, 2023
@sevu
Copy link
Member

sevu commented May 6, 2023

I think it does make sense to close it if the dirkfang takes the role of the hunter.

@CelticMinstrel
Copy link
Member Author

Maybe, but this one does have a pretty good portrait.

@doofus-01
Copy link
Member

If there are plans to work on the sprite, it makes sense to leave it open. Otherwise, closing it and linking an Issue makes sense, maybe someone else will pick it up. It's not like the portrait is getting tossed if this is closed.

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.

No sign that there is any work going on here, so I'm opposed to merging this into BfW 1.17/1.18. Maybe it can be a goal for BfW 1.19/1.20.

@Pentarctagon
Copy link
Member

Is this PR still being pursued?

@cooljeanius
Copy link
Contributor

Is this PR still being pursued?

Kinda 50/50 on this; in the add-ons where I might need a Naga Hunter, I've already copied it over, and thus I wouldn't really need this PR myself, but for new ones it might be useful. @CelticMinstrel WDYT?

@CelticMinstrel
Copy link
Member Author

I would still like this merged, yes.

@doofus-01
Copy link
Member

I'd say this is in the same boat as #7325; it could be a project for 1.191.20. Or maybe it could go into an "official add-on" if that's still being considered.

@spixi
Copy link
Contributor

spixi commented Apr 8, 2024

Maybe the portrait could be repurposed for a new line or the current Naga Hunter line could get a rework. The Dirkfang line is a classic mixed fighter line protecting the water frontline and the Hunter line could be more an assassin-like utility unit line with the high movement and poison special.

@cooljeanius
Copy link
Contributor

It seems that there are merge conflicts now?

@CelticMinstrel
Copy link
Member Author

Did someone update the naga hunter animations or something? Unless it's a case of move vs delete, the conflicts are probably trivial to resolve (just accept the upstream version).

@cooljeanius
Copy link
Contributor

Did someone update the naga hunter animations or something? Unless it's a case of move vs delete, the conflicts are probably trivial to resolve (just accept the upstream version).

I'm wondering if this is a side effect of doofus's updates to move all the copyright info to image metadata...

@CelticMinstrel
Copy link
Member Author

Ah yes, that's likely it.

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