Campaigns' TODO/FIXME list

[DisherProject]

Here i'm listing the TODOs and FIXMEs i've encountered while reading the files of some mainline campaigns. If you fix any of them, please tick the relative box and clear the line of the achieved TODO from the code.

The aim of this issue is just to point out the possible improvements that are not stated in the issue tracker, so that, once this is done, all of the pending enhancements can be found in one place (the issue tracker).

This list still isn't complete, i will add the missing campaigns once i'll finish reading their files. Let me know if i missed something.

Delfador's Memoirs

- [x] Missing portrait for Iliah-Malah ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Delfadors_Memoirs/utils/characters.cfg#L145)) - [x] This is about S19: a question about player's recruitment list ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Delfadors_Memoirs/utils/sides.cfg#L432)) - [x] S12 (i'm not sure of what this means) ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Delfadors_Memoirs/scenarios/12_Terror_at_the_Ford_of_Parthyn.cfg#L98)) - [x] S17 Fix to an enemy AI ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Delfadors_Memoirs/scenarios/17_A_New_Ally.cfg#L97-L108)) - [x] S21 Some not tested code ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Delfadors_Memoirs/scenarios/21_Clash_at_the_Manor.cfg#L303-L304)) - [x] S22 Doubt about giving the player bonus gold in the end (though this is the last scenario...) ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Delfadors_Memoirs/scenarios/22_Face_of_the_Enemy.cfg#L220))

Eastern Invasion

- [x] S2 Dialogue improvement ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Eastern_Invasion/scenarios/02_The_Escape_Tunnel.cfg#L236)) - [x] S9 Missing dialogue on victory ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Eastern_Invasion/scenarios/09_Xenophobia.cfg#L393)) - [x] S9 Missing winter scenery ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Eastern_Invasion/scenarios/09_Xenophobia.cfg#L164)) - [x] S10 Missing winter scenery ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Eastern_Invasion/scenarios/10_Lake_Vrug.cfg#L76))

Sceptre of Fire

- [x] Workaround for Dwarvish Miner's standing animation ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/units/Dwarvish_Miner.cfg#L45-L63)) (1*) - [x] S1 Missing winter scenery ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/1_A_Bargain_is_Struck.cfg#L128)) - [x] S2 Missing winter scenery ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/2_Closing_the_Gates.cfg#L66)) - [x] S3 Missing winter scenery ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/3_Searching_for_the_Runecrafter.cfg#L93)) - [x] S4 `STARTING_VILLAGES` macros may not work properly with random generated maps (?) ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/4_Gathering_Materials.cfg#L304-L306)) - [x] S4t Missing sound (Thursagan is shaping some gems) ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/4t_The_Jeweler.cfg#L202)) - [x] S5 Tell the player he can now recruit Gryphon Riders ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/5_Hills_of_the_Shorbear_Clan.cfg#L369-L371)) (2*) - [x] S6 Missing winter scenery ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/6_Towards_the_Caves.cfg#L166)) - [x] S6 Question about music ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/6_Towards_the_Caves.cfg#L405)) - [x] S9 Commented out dialogue ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/9_Caverns_of_Flame.cfg#L415)) (3*) - [x] S9 Objectives don't match the current victory condition (which is waiting for the volcano to erupt) ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/9_Caverns_of_Flame.cfg#L579-L580)) - [x] S9 Proposal for an improvement to enemy AI ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Sceptre_of_Fire/scenarios/9_Caverns_of_Flame.cfg#L632-L634))

Son of the Black Eye

- [x] S10 Missing dialogue on Lanbech's arrival ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Son_Of_The_Black_Eye/scenarios/10_Saving_Inarix.cfg#L685)) - [x] S11 Missing dialogue by Lanbec'h on start ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Son_Of_The_Black_Eye/scenarios/11_Clash_of_Armies.cfg#L302))

The Rise of Wesnoth

- [x] S17a Don't let the player go on fighting the saurians if time is over ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/The_Rise_Of_Wesnoth/scenarios/17a_The_Dragon.cfg#L520))

Descent into Darkness

- [x] S8 Missing scenery ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Descent_Into_Darkness/scenarios/08_A_Small_Favor2.cfg#L59)) - [x] S9 Missing scenery ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Descent_Into_Darkness/scenarios/09_A_Small_Favor3.cfg#L102))

Northern Rebirth

- [x] S1 Rebalancement of an event requested ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Northern_Rebirth/scenarios/01_Breaking_the_Chains.cfg#L319)) - [x] S2 Check location filter ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Northern_Rebirth/scenarios/02_01_Infested_Caves.cfg#L756)) (4*) - [x] S2 It is suggested to make the victory condition more robust ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Northern_Rebirth/scenarios/02_01_Infested_Caves.cfg#L905-L907)) - [x] S2 Restrict victory condition to Tallin having Hamel in sight ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Northern_Rebirth/scenarios/02_01_Infested_Caves.cfg#L908-L910)) - [x] S4 Someone should tell the player about necrophages' poisonous attacks if Camerin is dead ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Northern_Rebirth/scenarios/04_Clearing_the_Mines.cfg#L545)) - [x] S5 (a cryptic one :p ) ([here](https://github.com/wesnoth/wesnoth/blob/master/data/campaigns/Northern_Rebirth/scenarios/05a_01_The_Pursuit.cfg#L2277))

...

Some notes

(1*): I tried fixing this via WML. There are many ways to do it actually (e.g using variations, objects...), but it still felt to me like a workaround. I wonder if this could be done by working on the C++ (or Lua?) code. (2*): Imo, having the narrator say "You can now recruit Gryphon Riders" is not enough. Maybe, either Rugnur or Durston may ask whether the gryphons have been successfully tamed, and a gryphon rider may be spawned as a demonstration. (3*): I think fixing S9 in not very urgent. Imo, it may be reworked a bit, since it's just about sitting next to the glyph while the trolls are slaying elves, waiting for the volcano to erupt. If you agree with me that we could add some challenge to this scenario, i would say that such a thing may be achieved in many ways, so fixing the TODOs now could be useless. (4*): That filter looks good to me, so the relative TODO may be removed.

[GregoryLundberg]

There are a number of debuging commands. In general campaigns are designed to play through normally and ignore the issues which debugging might cause. The next-scenario command might cause problems but jumping around at random almost certainly does.

When I am working through a full campaign, I try to find ways to allow next to work rationally, especially with regard to units coming and going. Jumpto makes this much harder because you might be jumping backward with a unit which has advanced, or should not be present.

My recommendation: try to make it so next works, if you can. If you don't see a good approach to handling jump just ignore it (and remove comments like the one you found in DM).

When I'm debugging, I usually don't use either command. Instead I by-hand kill or move units using debug commands to set up the proper conditions in the normal flow (ie., long-move next to the oponent, set his HP to 1 and set mine to 999, then hit him for the sure-kill) so I can quick-forward to the point where I need to check

[sigurdfdragon]

I'll get to the DM ones hopefully by the end of the month

[sigurdfdragon]

The DM S12 jumpsafe fix is a bit involved. As such, it should be master only. Removed the portrait question. It seems unnecessary and unlikely.

Edit: I can't see a good approach to fixing the jumpsafe in DM, short of significant cleanup/overhaul, so I'll remove that comment at some point.

[Gtara]

What exactly do you mean by "missing winter scenery" in the first three scenarios of SoF?

[GregoryLundberg]

The links show you. It says # TODO: manually add snow detritus

[Gtara]

Hey, thanks for the quick answer. I followed the links but I still don't realise what exactly need to be done here. Are there any similar parts in other campaigns I could look up as a guideline?

[GregoryLundberg]

In HttT I used a Lua script to randomly place snow based upon how long it took to get through the previous scene.

You can probably lift the Lua code but will have to manually adjust the conversion table because it's specific to the HttT map. Since this is the first three scenes, you can't base it upon the length of the previous scene. But you could set the tuning value to either a constant or random value. If you do randomly place snow, I recommend my Lua because doing so with WML is glacially slow.

If you don't want to randomly place the snow, you can also hand-edit the map.

Be sure to play-test carefully, and have others do so. Snow slows down most units. This can be a death sentence if there is too much snow or if the scenario is timing-critical. Consider scene 1 of HttT. Most players would find it impossible to make the timed run to the corner if they had to crawl through more than a few tiles of snow.

Which leads to the final option: easiest of all simply remove the TODO.

[Gtara]

Hey again, I thought I'd work a bit on the map adding some snow(and a few ornaments here and there) manually. After one playthrough, the changes don't seem to alter the gameplay much. Maybe slowed me down a couple of turns but nothing seems to have changed to the extreme. Would anyone care to test it too? If this one's fine I could work on the other two as well :)

1_A_Bargain_is_Struck_2.zip

[GregoryLundberg]

It would be best to create a Pull Request so you can push changes, and the conversation can continue there instead of further cluttering this Issue.

[Gtara]

Hey I'd like to ask a couple of things regarding NR S01. First of all, what would you guys suggest as a rebalance of the troll event ? Something like allowing it to trigger after X turn for instance? Or maybe nerf it so it spawns less trolls? Also, a tiny detail, killing Al'Tar with a ranged attack will have the unit landing the killing blow say Stabs while they didn't kill him with a knife. Has anyone noticed this before/Is it the optimal behaviour?

[GregoryLundberg]

It's not asking for a rebalance, per se. I guess what it's suggesting is to make it less likely the player would go "Woah, where'd they come from?" That might be fixed with Shroud or Fog, or a map change moving the entrance a bit so the player can't actually see the units appearing. Or maybe pre-create them but prevent movement until the appropriate point.

I agree it's a bit of a problem when the dialog does not match the facts. It may be too much work to have it change based upon weapon type ("How does not stab with a club?!?") so you might want to see if you can simply change the dialog to be more generic.

[nemaara]

3860 deals with the DiD TODOs.

[nemaara]

https://github.com/wesnoth/wesnoth/compare/8b897e2adbb5...e37ce1eec1e3 fixes some of the NR issues.

[nemaara]

https://github.com/wesnoth/wesnoth/compare/7489f38114f9...363860c21db97d92a86dd053d0e9ba32982c7814 fixes the tomato surprise for NR S1, and removes the TODOs for S2, which are no longer relevant given that the scenario was slightly redesigned and rebalanced earlier so that chokepoints no longer really exist (so units die fast enough not to clog everything).

Some SoF issues, see: https://github.com/wesnoth/wesnoth/commit/7ddbdb9d6c29daecf1edcbcd94f819edfd62e3b1.

Sof S4 is not a problem anymore, I was able to place the starting village macro by the side definition without issue, but for this commit I left them in place and removed the comment.

More SoF issues: https://github.com/wesnoth/wesnoth/compare/7ddbdb9d6c29...da5e2b02a347. S6 doesn't need more snow.

S9 microAI: https://github.com/wesnoth/wesnoth/commit/65a737b937e0e0c64c719d66a5b4cd1bcf44bae2

EI: https://github.com/wesnoth/wesnoth/compare/da5e2b02a347...72cf97443993. S10 definitely doesn't need more snow or ice.

I've checked off all the relevant boxes for these.

[nemaara]

@DisherProject I don't understand what the problem is with the Miner's standing animation in SoF. Do you think you can elaborate or make a PR to fix the issue (since it looks like you may have already tried to address it)?

[DisherProject]

@nemaara As i recall, removing the standing animation block doesn't lead to a game crash anymore. Anyway, doing that will make the miner invisible (that is, he won't be displayed since he doesn't have any default standing animation), unless you trigger a combat animation or you make him carry either coal or gold.

Imo, any custom standing animation, like this, should not override the default one.

I did try solving this "issue" via [object]s, but it came out to be way too verbose. I left the FIXME there hoping someone could find a better solution than mine.

[nemaara]

https://github.com/wesnoth/wesnoth/commit/6265b63aa5280392a2e7aeb2f8676d7d74a245e9 should address the miner standing animation issue. The else block is still needed, if we want the default block to take over in the case that there are no other valid standing animations without that else block, that's an engine issue which can be dealt with separately.

Edit: looks like the [else] block is unnecessary, it works if you just have the [if] tag, so we don't need the extra [standing_anim] block anymore. See https://github.com/wesnoth/wesnoth/commit/d33a30a59286b4854c59a96f245f6926badc8f49.

[nemaara]

I'll put the final few issues in this comment.

EI: aa477cd SotBE: 2b5fc5b and f60f18c6, I removed the one for S11 because Lanbec'h already says things in S10 and there's enough text at the start of the scenario. TRoW: dabe47a SoF: 3ab9802 (removed the dialogue because that never gets fired anyway) 1fd767e, I chose to only put a narrator line stating that because having another scene after the initial dialogue really just distracts from the story

I think that's everything, closing this issue now.