Canik05 / freeciv-tnt

https://www.tacticsandtriumph.com
Other
3 stars 2 forks source link

Marines disembark to cap city #37

Closed Canik05 closed 2 years ago

Canik05 commented 2 years ago

as TnT's first game nears it's end I got several reports that "Your Marines were unable to Disembark." when trying to capture a city from a transport.

I checked the mp2-brava ruleset code, compared to fcw.org's. I see no differences. I checked units.ruleset and game.ruleset for the actionenabler.

TnT's ruleset code:

name = _("Marines") class = "LandAirSea" ... embarks = "Helicopter", "Land", "Sea", "LandRoad", "LandRail" disembarks = "Helicopter", "Land", "Sea", "LandRoad", "LandRail" targets = "Sea", "Balloon", "Helicopter", "LandRail" flags = "MultiSlot", "Marines", "FootSoldier", "BeachLander", "Capturer", "NeverBlocked", "Shield2Gold", "Settlers", "CanHide", "WillNever" ;Land units that are multipurpose for land/air/sea attacks (i.e., anti-aircraft, marines) ;include "Unreachable" in the targets for individual units [unitclasslandairsea] name = ("?unitclass:LandAirSea") min_speed = 1 hp_losspct = 0 flags = "TerrainSpeed", "DamageSlows", "CanOccupyCity", "BuildAnywhere", "CollectRansom", "ZOC", "CanFortify", "CanPillage", "TerrainDefense", "KillCitizen", "Airliftable", "AttackNonNative", "AttFromNonNative", "BorderPolice" helptext = ("\ • Can attack from Ships and Helicopters.\ ")

[actionenabler_disembark] action = "Transport Disembark" actor_reqs = { "type", "name", "range" "UnitState", "Transported", "Local" "MinMoveFrags", "1", "Local" }

I don't see an issue with it. I suspect this may have to do with changes to disembark in the last few months?

Canik05 commented 2 years ago

It seems to work in all other cases. A player just confirmed "BeachLander" is working properly too.

It's only when trying to conquer cities from transport.

kvilhaugsvik commented 2 years ago

Check your "Conquer City" action enablers. Do you allow conquer while transported / while on non native terrain / while on non livable terrain for Marines?

Canik05 commented 2 years ago

checking..

[actionenabler_conquer_city_marines] action = "Conquer City" actor_reqs = { "type", "name", "range", "present" "UnitClassFlag", "CanOccupyCity", "Local", TRUE "UnitFlag", "NonMil", "Local", FALSE "UnitState", "Transported", "Local", TRUE "DiplRel", "War", "Local", TRUE "MinMoveFrags", "1", "Local", TRUE "UnitFlag", "Marines", "Local", TRUE "UnitType", "Anti-Aircraft Artillery", "Local", FALSE ;AAA got Marines flag only to enable reasonable attacks, ;not for conquering cities from a Transport. } target_reqs = { "type", "name", "range", "present" "MaxUnitsOnTile", "0", "Local", TRUE }

"Conquer City" actionenablers are there for Marines and appear the same as fcw.org's dev branch.

I've looked through some freeciv/server/ files and javascript. action_dialog.js has differences but I don't think that is the problem.

kvilhaugsvik commented 2 years ago

The enabler looks sane. It is possible that the conquest is supposed to be illegal - say that the target city is occupied - and that the action not enabled explanation system is giving a confusing message.

kvilhaugsvik commented 2 years ago

Is this always a problem or is it sometimes possible to conquer a city with a Marine in a Transport?

Canik05 commented 2 years ago

It's always a problem. I replicated it in two singleplayer games. and in TnT Game 2 an alliance of 3 players all have said they can't conquer with marine via transport.

Canik05 commented 2 years ago

I went to try this on MP2-Caravel to see if it worked there and I got "Marines can't do Board".

Couldn't even load them onto a transport. I have not edited MP2-Caravel beyond fixing errors that prevented it's loading. This is how it was received from fcw.org who did not have it's full or true source uploaded for the longest time. It was full of such errors.

kvilhaugsvik commented 2 years ago

Could you check a Freeciv rulesets that works on web like classic or civ2civ3? If Marines Conquest is broken there too (and they aren't modified) we know that we aren't dealing with a ruleset bug.

Could you also try if the act sel dlg (press 'd', click target tile) allows you to Conquer an adjacent city from a transport?

kvilhaugsvik commented 2 years ago

The board issue is probably a separate issue. Check if you have enablers for board.

Canik05 commented 2 years ago

Ok, I tried on Classic and they were unable to disembark to capture a city there as well.

kvilhaugsvik commented 2 years ago

So probably not a ruleset problem - assuming that classic isn't modified. Some ideas: I can't remember fixing something similar upstream but have a look at the Freeciv change log for the commits after your version and/or of a diff between Freeciv upstream actions.c of the Freeeciv revision you use and your actions.c if you don't have a better clue to follow. Another possible help is to look at your server logs to see if the problem show up there. You could also try to craft and send the packet sent by the action selection dialog by hand to see if the JS client somehow sends the wrong data. There may be better ways to debug. I'm not used to working on freecivweb.org or derived stuff so I'm not the best person to help you.

Canik05 commented 2 years ago

I looked at actions.c

fcw.org & tnt.com actions.c files were almost the same. I brought over fcw.org's actions.c file, tested, it did not fix the issue.

freeciv/freeciv's actions file is quite different.

I checked server logs, nothing there. I will download install.log and check there?

Canik05 commented 2 years ago

I downloaded the install.log.

I searched for actions, conquer, disembark.. didn't see anything helpful.

kvilhaugsvik commented 2 years ago

I ran a test on the current Freeciv master. My Marine in a Transport were able to do Conquer City from non native in classic but only after I removed its "sentry" activity. My current C client set up doesn't remove unit activity unless I try to move the unit or explicitly remove it. Can you confirm that your marines aren't sentried?

kvilhaugsvik commented 2 years ago

I'm talking about server logs, not the install log. Example: logs/freeciv-web-log-6000.log

Canik05 commented 2 years ago

I'm talking about server logs, not the install log. Example: logs/freeciv-web-log-6000.log

ah, yeah. I checked them first. there was nothing concerning the issue.

I ran a test on the current Freeciv master. My Marine in a Transport were able to do Conquer City from non native in classic but only after I removed its "sentry" activity. My current C client set up doesn't remove unit activity unless I try to move the unit or explicitly remove it. Can you confirm that your marines aren't sentried?

I tried making sure it was not sentry, it still could not conquer. Moving a unit on web automatically removes sentry. I don't think is the issue.

However while testing that.. I tried attacking an explorer from my transport and I got the same error. (Unable to Disembark)

So the bug is not limited to conquer city, it seems.

Canik05 commented 2 years ago

I checked server logs again. Still see nothing concerning it.

Earlier today I looked at diff comparisons TnT missed out on so many updates. I will go through it more thoroughly tomorrow.

kvilhaugsvik commented 2 years ago

Could it be that you are sending a disembark action every time a unit is in a transport even if only Conquer or Attack is possible? Try using the action selection dialog (or sending a packet by hand in case even the action selection dialog gets rewritten).

Canik05 commented 2 years ago

@kvilhaugsvik ok yes! hitting d then selecting the tile worked.

so the issue is it is trying to disembark when it should be a conquer/attack action only.

kvilhaugsvik commented 2 years ago

It looks like 15eb3819d63e3b1ed5cc17d65cf0b4e9f006e849 reverted a bunch of stuff from fcw.org. Intentional? I think you may have reverted the fix to this issue because I remember it now...

Canik05 commented 2 years ago

It looks like 15eb381 reverted a bunch of stuff from fcw.org. Intentional? I think you may have reverted the fix to this issue because I remember it now...

When I experienced issues with fcw.org's code I did switch to a slightly older version to base TnT upon.

I probably should have explained that sooner, sorry. It feels like forever ago now. :D

If you remember it, could you point me to the fix please?

kvilhaugsvik commented 2 years ago

This is how the fix looks in Freeciv-web: https://github.com/freeciv/freeciv-web/commit/53749adc2e3ed80a412e2bac7bcec92107df87a6 Look for something similar in what you reverted to find the proper way to fix it in your fork. (It may be identical but there may have been issues that needed porting.)

kvilhaugsvik commented 2 years ago

If you think the fix will work as is you can cherry pick it with: git fetch git://github.com/freeciv/freeciv-web.git # downloads Freeciv-web to your repo, including 53749ad git cherry-pick -x 53749adc2e3ed80a412e2bac7bcec92107df87a6 # copy the commit If something went wrong, you can see it by git complaining git cherry-pick --abort

Canik05 commented 2 years ago

This is how the fix looks in Freeciv-web: freeciv/freeciv-web@53749ad Look for something similar in what you reverted to find the proper way to fix it in your fork. (It may be identical but there may have been issues that needed porting.)

In a test this fixed the issue. I will upload the change the github and include it in the next TnT rebuild.

Thanks, Sveinung!