How do we want Monolith's to work for players and VCAI?

Perhaps just don’t add such uncertain options? In some situations player may want to move through dangers (e.g. battles), sometimes - he may want to visit “bad” objects, etc. While you wish to add option that basically says “nah, I don’t care if you’re OK with visiting this - I said that you won’t go through here”.

Instead I think that we should have:

  1. Normal pathfinding - it will only go through some locations like owned/empty garrisons & two-way teleports. That’s it - 100% safe options.
  2. Extended pathfinding, used via some modifier key (ctrl?). It will go thorough anything - neutrals, enemy-held objects, any other objects on the way. It’s purpose would be to show presence of path from A to B so player may check his options, not to build a safe path. Players will be able to use it to find some way into location blocked in one way or another.

But players don’t need to get automatic pathfinder that goes through dangerous objects. AI does, because it is stupid.

Currently there’s SectorMap in VCAI that takes care of guarded tiles and removable objects, however has its limits. For example it cannot find a path through guarded tile which is on the other side of Subterranean Gate, which can lead to infinite loops in some scenarios. It eventually should be merged with updated pathfinder, but this may take time and will be tricky.

However, SectorMap also handles some additional situations, such as buying ship for AI. So there’s no direct translation between pathfinder and AI decisions.

To be fair I don’t get how is this option uncertain. Who care if player go through Garden of Revelation, Learning Stone or Lighthouse without visiting them? :unamused:

In same time we can disable it for any objects that give temporary bonuses to hero, negative or positive. E.g any objects that have visit limit so player don’t “cheat” by not visiting them.

PS: And yeah as I said before I wish to implement something more complex in future so player may be able to choose how it’s work, but for now I think it’s more than enough to introduce such option as long as it’s not going to break original game gameplay rules.

Another little update. In order to manage enablement of one-way monoliths I’m started to move more logic into CGameInfoCallback. Though for now it’s not work exactly as I expected as my callback functions are called from within of CGTeleport, but even on client there is no player-specific “cb” available and I can’t check if player see object or no.

UPD: Sadly it’s even more complicated as it’s looks like “CPathfinder::calculatePaths” don’t have access to player-specific gamestate too… :neutral_face:

So I’m find few tricks to bypass that problem with player state. Not too pretty, but works.

Also I just implemented teleport channel exploration support for the VCAI. Now if AI hero going through bidirectional teleport channel and notice there is available exits which isn’t visible yet hero will instantly try to visit each of them and then teleport back to prior position.

So this also going to help the AI in situation of vision loss due to fog of war changes. E.g if channel have teleporters “A B C D” and for some reason vision on “B” was lost then even if hero just going from “A” to “C” he’ll check “B” too.

Improved teleport exploration of VCAI so:
[ul]]AI will remember blocked (no exit) teleport channels and will never attempt to visit entrances of such channel again./:m]
]AI with Explore goal will visit two-way teleport object (even if same entry was already visited) if any of exits of it’s channel are under FoW./:m][/ul]
Also find out about interesting H3 behaviours:
[ul]]If one exit blocked by friendly hero blocked then it’s will always teleport to different exits/:m]
]If all exits are blocked then it’s just do nothing. /:m]
]But for Subterranean Gate it’s will initiate exchange between heroes.
Also this exchange not using any movement points./
:m][/ul]For some reason before I’m think like last behavior works for two-way Monolith’s too at least if they only have two exits…

Any thoughts on how this should be implemented in VCMI?

So I’m implemented almost all H3 behaviours except free hero exchange and added nice bonus feature to CPathfinder. As I posted before there is two separate options for unidirectional channels:
[ul]]allowTeleportOneWay: use channels with one known exit/:m]
]allowTeleportOneWayRandom: use channels with more than one known exit/:m][/ul]Now if there unidirectional channel with two exits and one exit is blocked by friendly hero pathfinder will treat it as channel with one exit and will build paths through it.

It’s would be good base to implement unidirectional teleporters exploration for the VCAI in future. E.g AI can leave heroes to stay on unidirectional channel exits while probe entrance using more heroes so it’s can check how many exits is there for sure.

PS: All new code is little bit hacky again, but I’ll go through few improvements passes once I’m sure that everything implemented.

Finally had time to test some behaviours @Ivan posted earlier.
Tested on map without taverns and towns.

In this in H3 it’s saying you “Unfortunately, it appears that the owner has left some guards and in VCMI behaviour is same. Do you wish to fight them?”. So it’s impossible to attack it accidentally.

In H3 it’s doesn’t show stacks in garrison, but does show “attack” cursor on it.

So it’s questionable if these are bugs in H3 or not and I have no idea what is valid way to implement it.

PS: Anyway if @Ivan still think this shouldn’t be implemented then I’m fine about that as it’s can be always merged separately later on and this will save me from digging into object creation process. In this case I’ll just add some teleport-specific code that allow/disallow transit.

I think you try to pack way too many features in one branch and it becomes very confusing. First finish one thing, test if it works and only then continue.

Let’s say fair I’m don’t really want to do that, but in same time I don’t want to merge code that going to break more things than improve. E.g transit feature appear not because “it’s cool and I want to do that”, but because there was real issue with accidental teleport visits that have to be solved. Same story about almost everything else.

Only thing I did “for fun only” was this Subterranean Gates for multiple layers, but I’m already removed that code for now. And I’m think I can leave very minimal transit code for teleporters only too.

Also I actually do test all code every time I rewrite anything.

One more problem with putting more things there is saves compatibility. I’m wish to avoid future save format breakage while I’ll implement other pathfinding improvements.

Though if you think it’s will be okay to break saves with next merge too then I may stop at this point and leave some duplicated code in VCAI for now. E.g don’t remove knownSubterraneanGates, leave knownTeleportChannels in VCAI.

Then I can probably cleanup it and sent pull request in day or two.

So I started to get code ready to pull/merge.
Hope pull request will be ready tomorrow.

Pull request is up: #93

Want to say huge thanks to @AVS as this code wouldn’t be done without tons of time he spent helping me. Would be fair to say it’s for 50% his code. :wink:

@Warmonger @Ivan also really appreciate your comments and help!

There is one more important issue. While pathfinder can access portals and gates, there is also SectorMap class in VCAI. Its purpose is to find way to objects blocked by removable treasures or monsters - very important thing. Also, SectorMap handles Boat construction, which is very important in some scenarios (possibly bugs.vcmi.eu/view.php?id=2151 ).

It eventually nees to be merged (unified?) with Pathfinder, or at least make excessive use of it. There are several cases not handled last time I tried, such as accessing guarded tiles on the other side of Subterranean Gate (bug bugs.vcmi.eu/view.php?id=2119 ) and similiar.

I was busy for about two weeks, but not have free time again. I doubt that #2119 related to that, but I’ll look into it. Did some experiments to improve movement code internally, but it’s will likely break saves so that clearly won’t go into 0.98.

Though I’m really want to fix this one before 0.98:
bugs.vcmi.eu/view.php?id=2134

Time to bump that old topic.

@Warmonger what do you think do we really need subterranean gate code in SectorMap atm? Any reason to not disable it for now?

I just start to dig around #2119 issue and it’s not the only problem that code cause.
It’s will take a while before I learn how to design AI-related code properly so may be it’s good idea to comment it out as I not sure it’s doing anything helpful at all.

Also I’m like concept of SectorMap overall and think it’s would be right tool to use in situation when it’s not worth to check every possible movement type performance-wise (e.g using Town Portal spell or Dimension Door).

The SectorMap is here to check if a movement would be possible over removable objects (monsters or treasures). Otherwise it should duplicate Pathfinder and is superflouous in this role. If we could handle Subterranean Gates with Pathfinder instead of SectorMap, it would be great.

Also, don’t forget that SectorMap also handles Shipyards. This piece of code looks a bit off (causes AI action instead of returning path/tile info).

Yeah I’m seen shipyard code and SectorMap is looks good location for it. Though for me it’s looks like VCAI can do just fine without Subterranean gates code and I’m currently running AI with that code commented out:
gist.github.com/ArseniyShestako … c54ebf7961

It’s looks like VCAI is doing okay without it so I feel we may want to comment it out in develop at least before it’s fixed. I think like unexpected loop is far worse issue to keep it in develop.

I just run several games with AI simultaneously at moment and it’s looks like bug with subterranean gate loop is the only serious game-breaking issue remaining.

So I working around that code today for several hours and I don’t think there easy or fast fix for this. Appropriate support would take several weeks to implement and test. :frowning:

For now I disabled it until I at least drop all legacy code and rewrite it to use teleport channels. And probably at least some guards handling have to be done in pathfinder.
github.com/vcmi/vcmi/commit/975 … a58bd7e270

I’ll definitely work on this in future, but at moment it’s just make it harder to test AI and find other important bugs. Considering it’s only worked for subterranean gate anyway I don’t think this will make AI any worse.