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

International board Development
#21

It’s all looks trivial for me until I actually try to write some code that works and not looks like crap. E.g I still need to understand how to handle if certain player see object on server side and how to not expose data player shouldn’t have access to.

Of course I’ll find out, but it’s take time to learn how engine works. :blush:

#22

So here is my brand new CGMonolith code, any opinions appreciated:
github.com/ArseniyShestakov/vcm … efactoring
It’s should allow to decrease complexity of teleporters handling in pathfinding code a lot.

Though now I’ll will start to work on teleport select via query. :slight_smile:

#23

Small suggestion - you can open pull request and ask for review. In this way we can easily use all code review features on github. Right now your code is split across 3 commits - easy to access unified diff would help a bit with review.

#24

Ok here you go. There is teleport-only:
github.com/ArseniyShestakov/vcmi/pull/1 (I closed comments in this one)
And there is same teleport code merged with pathing improvements (WIP):
github.com/ArseniyShestakov/vcmi/pull/2

Though I’ll only sent pull request to mainline once everything is ready include ability to choose teleports (just start working on it). And possible that I’ll rewrite this one more time.

#25

So I finished basic implementation of monolith chooser based on BlockDialog code (@Ivan thanks for suggestion). Though at moment I need to find a way to pass needed teleporter ID into “CPlayerInterface::showMonolithDialog”. E.g it’s looks like it’s kind a not in sync with doMoveHero. :blush:

Though once it’s done I’ll only need to squash one quirk. Currently there is problem that if hero staying next to Monolith from left or right it’s and it’s need to go in that direction it’s will go into Monolith.

So I need to make hero to get around these small Monoliths in this case.

UPD: Pushed new destination chooser code! Now it’s work!

#26

It’s not the issue of monolith only. If map objects have mask of “VVVAVVV”, path chooser lays path through A. It needs to be corrected for all objects (suppose to just check if an A hex on the way, and lay path around).

#27

As far as I seen it’s not problem for other objects as currently pathfinder only use tiles that marked as ACCESSIBLE and not VISITABLE. For teleporters I’m of course need to ignore this while they’re VISITABLE. So it’s where problems begin. :confused:

#28

Started work to get rid of static data and now teleportChannels is stored in gamestate->map.
Though met weird problem with findMeChannel function as it’s currently may return some dead pointer. :frowning:

PS: And yeah I’ll likely move generic functionality from CGMonolith back into CGTeleport, but it’s won’t be used as handler, but only as base for other handlers. This will help to keep code a bit more clean.

#29

Also I just got new idea from my friend. So now I’m like to make my code reusable for both Castle Portal in Inferno and Town Portal spell. I think pathfinding via Inferno’s Castle Portal would be great feature.

#30

The idea is interesting but I expect speed problems.

#31

Why? Pathfinder already runs over entire map so increase in accessible territory (due to town entrances usage) would rarely change. Finding all possible town portal targets should be also quite fast especially if such list would be pre-generated as extra teleport channels (one for each team).

However I’m not sure if this is a good idea. Infernos’ Castle Gates are one thing with no downsides I know about but spells will result in not so obvious spending of mana points - what if player wants to keep his mana intact? Making this toggleable is not an option - this will only mean that players will have to constantly switch it on and off depending on hero.

#32

Enable by holding hotkey.

#33

I’m not yet talking about enabling automatic spellcasting within pathfinder. For now I just wanted to make my pathfinder code reusable for Inferno Gates and because this object currently use mechanics of Town Portal that spell should be also handled by CGTeleport (new-old name of base class as I split it and CGMonolith).

Though I already think about automatic spellcasting as future feature because it’s would be great help for AI, but I decide to not start work on any new functionality until current code is ready for merge. And after all flying and water walking is a lot more important than other things.

#34

More news today:
1 - First of all there is incomplete implementation of “transit” option available. This is allow pathfinder to build paths via “VISITABLE” objects so hero will go through them without visiting. Of course it’s won’t be possible to transit through events, one-way monolith entries and whirlpools (as long as hero don’t have protection or only have one stack with one unit).

Though currently option denyTransit in JSON is missing as it’s end up being tricky to add new options. E.g currently I wanted to put it into “appearance” via “base” setting, but there is problem that “base” section in JSON only parsed if subtype in JSON also include “templates” while they using one from H3 assets.
…
2 - I finally fixed sync between event handler and doMoveHero thread, but there is few consequences:

    • First of all now movements via teleporter should always go via TeleportDialog.
    • Secondly I temporarily commented out whirlpool-specific code that now have to be called from teleportDialogAnswered, but I don’t want to just copy-paste code there.

3 - Added option allowTeleportOneWayRandom to pathfinder. So now pathfinder client may also attempt to use unidirectional channel with multiple exits by choosing needed exit and hope random will work as needed. :laughing:

4 - I also started to write huge super-detailed overview on pathfinding and movement:
wiki.vcmi.eu/index.php?title=Use … d_movement

So not yet solved problems are:

  • Find way to make denyTransit option in JSON works.
  • Need codepath to avoid accidental visiting of one way monolith entrances.
  • Re-enable whirlpools specific code.
  • [UPD] Need to find out with one case when doMoveHero still desync. :frowning:
  • [UPD] Of course need also find out with getNeighbours and get rid of hack in CMap.cpp

Now only big thing remain is VCAI code.

#35

Good idea. Although this would still need some sort of mana spending visualization like current number of turns to target.

Still I don’t think that such functionality would be often used by players and AI would be better with separate mana spending logic instead of placing this in pathfinding.
(although one-way monolith movement is a good idea for AI. Assuming that AI can handle one-way paths)

It should be not in appearance but in subtype description (one level higher). Appearance should only describe how object looks on map. Everything else should be in object config.

That’s correct. Server does not cares whether hero is visiting monolith or passing through it.

Don’t like things like outTeleportObj/tileAfterThis members though… I’ll try to go through that code to find some nicer solutions to handle this. doMoveHero would be a better place without them.

#36

I think that pathfinding may suggest you to do that, but not enforce that choice. From my point of view it’s would be perfect if you can click on any point on map and pathfinder will suggest the path through pickable objects, guards (for example if they weaker than you), water, etc. As @AVS suggested it’s may be something like Ctrl+click or Shift+click.

It’s doesn’t mean that everyone will want to use it every time, but it’s would be really useful because somethings it’s quite hard to find entry of some area of find what monster is blicking entry.

In case of AI it’s may be useful in case if goal is to get grail or certain artifact.

Got it. Though any example would be helpful.
Let’s say fair object loading code is quite complicated.

All code in doMoveHero is really hacky at moment. I’m wanna rewrite it as well as CPathfinder, but I think this should be done only once all functionality implemented. New movement system (one we discussed on github), water walking and flying going to add even more complexity here.

So I think it’s better to find out all use cases and only then rewrite it.

#37

Yeah. On the other hand - this is as simple as I could get it without completely breaking H3 behavior.
wiki.vcmi.eu/index.php?title=Object_Format

Idea is that

  1. There is object group always using same handler class
  2. Object group contains list of object types and “base” entry to save typing for common entries.
  3. Each object type contains list of templates and “base” entry. ( and this “base” may be inherited from object group “base” entry)
  4. If there are no defined templates - VCMI will try to import this data from H3

For your case you need something like this:

"someMonolith" : {
	"index" : 12345,		// unique ID
	"handler": "teleport",	// handler ID
	"base" : {				// entry that will be added to everything in "types", optional
		"myCoolEntry" : "something", 
		
		"base" : {			// entry that will be added to everything in "templates", optional
			"visitableFrom" :  "---", "+++", "+++" ],
		}
	},
	"types" : {
		"sometype" : {
			"index" : 0		// object subtype
							// no "myCoolEntry" here - it will be inherited from "base"
		},
		"anothertype" : {
			"index" : 1,	
			"myCoolEntry" : "not something" // or if you want - you can override this on per-object basis (or set it in each object and forget about "base")
			}
		}
	}
},

In this example object handler will be created twice, once for “sometype” and another for “anothertype”. But in both case it will receive “myCoolEntry” field - before passing data to handler VCMI will merge object type description with base entry.

IIRC you’ll need to create custom handler for your class - generic handler can only be used for properly-free classes.

#38

Thanks. Will do next attempt to implement it. Though only thing I worry about is fact that I don’t want to replace H3 templates.

#39

You don’t have to. This is one of ideas behind this structure.

If your object does not provides “templates” entry VCMI will try to import them from H3. Structure from example above will work fine - types don’t have templates defined so it won’t break h3 objects

UPD:
Why do we have that loop in doMoveHero?

Perhaps a better approach would be something like this:

  1. player requests hero movement -> client sends moveHero request & blocks UI
  2. client receives reply from server -> moves hero according to reply
  3. if path is not empty and interruption was not requested -> send moveHero again
  4. Goto 2

So instead of such loop there would be ping-pong between server and client till player will request stop or hero reaches his destination/out of MP.

Oh - and as for “extended pathfinding” - IF you’re going to implement it then it should use any possible means of travel including battles ( passing through enemy heroes & neutrals) in addition to things like spells/one-way monoliths. Main purpose would be not to choose route but to show player possible way to reach destination.

#40

One way monoliths is already working and there really nothing wrong with using them as long as we know there only one exit. Everything else is just idea that may be implemented, but only after more important things like flying/water walking are ready.