search

konecnyjakub / Danse_Macabre

User made campaign for Battle for Wesnoth. Originally created by kamikaze
https://forums.wesnoth.org/viewtopic.php?f=8&t=33745
GNU General Public License v2.0
4 stars 1 forks source link

drake/gryphon die animation #20

Closed sevu closed 7 years ago

sevu commented 7 years ago

Not working as intended. I will have a look at the WML later.

New base images for the higher levels would as well be a solution.

konecnyjakub commented 7 years ago

Can you still reproduce this? Especially after merging that PR? If not, we should close this.

Also, can you elaborate on the new base images part? Are you talking about changed sprites for Walking Corpse/Soulless gryphon/drake on Wesnoth 1.13 or something else?

sevu commented 7 years ago

I wrote an animation for the drake/gryphon which looked like the default fadeout. It missed an __remove=yes to not display the die5~10 images after fadeout, though with it I encountered the bug in wesnoth. I tried pushing a 1.12 like version to master, but seems I can't push to master. Found a workaround for the bug afterwards.

In 1.13 the new drake, wolf and probably gryphon images look good using the same style of die animation like the other ones. But it's not high on my list to work on that now.

konecnyjakub commented 7 years ago

I tried pushing a 1.12 like version to master, but seems I can't push to master.

I am surprised to hear that since you are a collaborator now. Master branch is protected but it should just prevent everyone from force-pushing to/deleting it. Anyway, I think you should be able to create new branches in this repository.

sevu commented 7 years ago

I submitted two branches, have a look.

As for master

remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: At least one approved review is required by reviewers with write access.
To github.com:konecnyjakub/Danse_Macabre.git
 ! [remote rejected] master -> master (protected branch hook declined)
konecnyjakub commented 7 years ago

Thanks for the error message, it is really interesting. So the checks (see screenshot below) means something different than I thought. That probably means that only I can push to master. Pity. Branch protection

konecnyjakub commented 7 years ago

I have quickly checked both branches in-game (on 1.13) but I fail to see what are you trying to fix ...

sevu commented 7 years ago

In 1.13 the drake die animation is a fadeout. In 1.12 it looks in effect the same. Reason is, I feel the change from the drake to the die[5~10] images looks buggy, but in 1.12 mainline does it like that too, so I went the same way.

konecnyjakub commented 7 years ago

So, you want to be consistent with mainline? I guess we should do this then even if it does not look much better (or any better imo). The animation style has changed in 1.13? That is why there are 2 branches, right? Well, it might be better to have just 1 and guard the differences with #ifver. Other (a bit related) stuff: I am not sure if we should upload 1.3 (once it is ready) for Wesnoth 1.13 as the sprites of custom zombies would not match the mainline style. Any opinion here?

sevu commented 7 years ago

Using #ifver sounds good like a good idea. I think we'll not get new sprites anytime soon... So I would upload it to 1.13 too.

konecnyjakub commented 7 years ago

Well, there is Art Workshop at the forums and we are talking basically about recoloring sprites. Anyway, thank you for your opinion.

sevu commented 7 years ago

Art workshop is a great idea. I just see I skipped some of you questions the last time. Animation style didn't change in 1.13, but I think the switch in the 1.12 drake die animation after the first three images to the remaining ones doesn't fit well. I first thought it would be a bug that I introduced ... To keep consistent with 1.12 mainline the change is only applied for 1.13 (and a very minor change to the 1.12 one with the alpha key), but if you prefer you can drop that.

I ran a script, originally written for for ageless era, to use mainline-strings when possible here too. I think it can be released.

Pushed it to the 1.13 branch, could you remove the 2nd option on the screenshot above?

konecnyjakub commented 7 years ago

I ran a script, originally written for for ageless era

Is it anywhere online to see? I might try on my own campaigns, it have done a good job here.

I think it can be released.

I am not sure what are you referring to here. That script, new version of this campaign, something else?

could you remove the 2nd option on the screenshot above?

I have enabled for additional quality assurance but as it is causing some troubles, I can disable it. It is not that important to have it enabled.

I am going to cherry-pick the commits to master tomorrow, after some testing.

konecnyjakub commented 7 years ago

The 3 commits are in master. I also disabled the second check so you should be able to push to master.

sevu commented 7 years ago

Thanks. Here is the script: https://gist.github.com/sevu/ae3034373d1b48617967be2e5b3225e8 Be careful, if you run it without arguments it replaces everything in the subdirectories. (happened to me^^) Ageless has a mainline-strings file which contains them all. What about releasing the add-on?

konecnyjakub commented 7 years ago

I would like to release 1.3 as soon as possible but I am not sure if it it ready. The only (possible) blocker for next version I know of, is the mismatch in style of sprites for zombies. But it is probably not so serious issue.

sevu commented 7 years ago

Thought that only applies to 1.13. It could be delivered as 1.4.

konecnyjakub commented 7 years ago

Yes, that is exactly why I do not consider it so big issue. So unless something important emerges, I am going to release 1.3 tomorrow.