qsa

Pushed few arbitrary changes. Feel free testing

The main reason for much bigger values are the stacking auras. While this can also affect the final values, its effect is less that 1% overall. Still nice thing to have tho. Thanks.

and applied this to the latest core. I then applied qsa's patch and then I built it. This is what I get; Pet Follow Automatically Hunter pet yes Warlock minion no Companion no It looks just about right. But I have no idea what you are doing wrong, or what the problem really is. I tried it once again, just to be triple sure, and it works for all types of pets. Tested on both water elemental and succubus. My tests are done on our repository + that patch. Maybe you have problem with merging or applying the patch. Or maybe, its problem with other custom patches you use. It also can be addon or something like that. The reason I'm so sure there's no problem, is that both hunter pets and lock pets use exactly the same movement code. Best regards.

Just tested, and it works like a charm for me. So I cannot confirm this report.

Right now, pets will try to come as close as they can to target/owner. I think its more than reasonable. So, no changes in pet code needed. Don't really remember how it is dealt with on retail, but its either that, or just taking no action at all. Personally, I like the first option. It doesn't cost us nothing. I'll give people time test it bit more, and push it.

Thanks for testing. This issue should be fixed in my latest test patch. Feel free retesting and report. faramir118: Are you sure http://github.com/faramir118/mangos/commit/27980025db926e26fedf758cf1d4479f321a3386 is necessary? those modes are far more useful than the debug. Simply since you can "play" in real time with parameters and see the effects.

Below attached test patch. Implements evading and aggro changes on targets that become unreachable. Pretty much everything we spoke about. It uses the lazy logic. Which makes it simple yet effective. The only thing missing (as far as I know) from all the things we spoke about, is evading timer. Right now its 0 delay. But since it is used only in very specific place, this feature can wait for later. Feel free to test and report issues. Patched over (c8a25250df7752) :: http://pastie.org/1248321 Ideas, remarks, criticism is always welcome. Take care. PS: Can anyone check that I remove the target from aggro list properly? lines 140 and 200 in diff . Thanks in advance.

Can confirm this one. Not very common, but happens from time to time. Can post nice collection of dumps, but they are all same as above.

About that patch: we are not yet on mangos version that require this change. It will be included when we update. About DoStartMovement : is is unforeseen side effect (when offset is used) from old evade code. I am currently working on implementing better way. This issue should be solved there. I'll post it for testing, as soon as I have time for some basic checks. Thanks. PS: do you have issues with stability?

This stack looks more logical, but, without line numbers and locals there's not much I can do. Either that, or way to reproduce. I don't see how it can crash, but sure, there always can be really strange corner cases. So, ye, in a way, debug can solve the crash. Just as others already mentioned. If anyone have *nix core dumps, don't be shy and post them here. If it is as common as KiriX say it is, you shouldn't have problem getting those. Take care.

SD2 related? I can bet this is due to spawning Jaina outside hyjal, thus m_pInstance is empty, and it isn't always checked before use in scripts.

This is not valid stack call. Any chance you know how this can be reproduced? Thanks in advance. PS: anyone having any other related crashes? ( with *nix dumps preferably )

Its not real solution. Tho we can add it as temp hack. Not sure if we should deal with this at all. But in general, it can be good idea to have the ability to limit the point path length dynamically. Thanks for noticing this typo. I pushed the change into repo. We welcome every little bit of help.

It will "always" get you to destination (unless there's problem with mesh). I never seen "path error" on retail, maybe its since LoS checks cover almost all such cases. But from what I remember, you can charge across small gaps. If we going to have the option to specify parameters for each and every specific map, maybe we can make bash script with all those "best" parameters for every map. Some community input can be nice here. which values work the best for you for every map, etc. PS: I pushed my recent changes into the repo.

It's like 5min of "work" splitting them and pushing (unless you do it manually, which I hope for you sake, you don't). If you going to add me, I can push my changes on my own.

heh, thanks for showing. Ye, I aware of this kind of bugs, but unfortunately there isn't much I can do there right now. The problem is with the mesh itself. I bet that step right there is blocking the path, so all that longer path is found instead. You can check it by generating debug info while generating mmaps, then opening it with recastDemo and actually seeing all the walkable polygons. The reason we cannot eliminate those things all together, is the same reason we still have sometimes bugs with vmaps ( LoS checks). There are just too many cases. We can't make it manually either All we can do is try to minimize those. Maybe faramir118 or Gotisch can take a look at those. Since I haven't really spent time on the generator yet, thus don't fully understand it (yet). In general this charge shouldn't be casted at all due to LoS fail. But, as I said in last post : LoS+terrain = bugs (at the moment atleast ). I did my tests in BE arena, and it worked there pretty fine. Take care.

I rather not, but if you really don't have time dealing with this, I'll add it. My nick there is sixsixnine . Firstly, thanks for testing. About the issue you describe: The real problem there is the value returned from GetContactPoint(). Target falls under map since point returned there ( in some cases ) below the actual terrain. This is due to VMAPs. The way point path is generated for charge effects, is that it will always reach the final point given.. I do know this issue exists, but it is not directly related to mmap. Therefore, I think, all those changes and problems should be solved regardless of pathfinding usage. For example, the last case scenario (and first) you gave has absolutely nothing to do with mmaps at all. It should be handled in LOS checks. But as we all know, at the moment terrain in not considered as obstacle in LOS calculation. Just to sum it all up : someone should make independent patch solving those issues. I rather not do it as part of mmaps. Simply since it is not directly related and we don't want to make this project even bigger than it currently is. Best regards.

Made few more changes. * Everything from my previous patch ( post #445) * Ability to specify which type of point path to generate. Either smooth or straight. * Implement pathfinding in charge effects using straight path. - Added Unit::MonsterMoveByPath() to simplify this case and allow usage in similar. * added the ability to specify path type in ".debug mmap path" command. ".debug mmap path true" for straight path, smooth is default (or false). patched over (213e909001b7) :: http://pastie.org/1232897 Take care.

Thanks for testing. This is not exactly how I remembered it, but close enough. Are you sure that if you are unreachable target will not even try to come closer? Overall this seems not very natural behavior to me. Don't really sure why. But its clear that "they" deal with "no full path"="something wrong" -> just error out ( evade ). Can you please also test the scenario when you have 2 targets. Main target becomes unreachable, while the other still is. NPC will switch to 2nd target, but what happens with threat? something visible? Thanks in advance. Should we follow this type of behavior at all? I mean, do we really need to copy their bugs? We can either try to mimic retail behavior using the fastest/most accurate solution, OR we can try to make something more reasonable. But that's totally different question. I think we disagree on the "how", not the purpose of pathfinding. We can always make different class with only purpose will be checking reachablity. We sure can save some CPU cycles there, since we wont have to generate anything but poly paths + couple simple checks. Then we can use this class for checking targets in SelectHostileTarget. This is perfect solution. We'd know if we can reach target even before selecting it. Unfortunately, this is where we really disagree. Perfect solution do not exist in our imperfect world. Those checks are not free. If they were, we'd just use them. Here is where the problems start. Making it happen. If we just check all those targets and then generate path for chosen one we are wasting alot of resources. If we try to somehow use the data we generated there to build the path, we are mixing AI with Movement. Sure, you say : "lets make it so efficent, so it wont metter how many paths we generate, how many checks we run". The best you can do here is cut the overhead as close as you can to A* cost, its our lower bound. It is far, far from "free" or low. Best regards

I'm almost 100% sure they do it by either dropping unreachable target or reducing its threat by %. So we automatically have new target to attack, hoping we can get to him. Maybe someone can test this on retail? There is no freaking way they generate paths to all targets at target selection. We can do the same: no path -> SelectHostileTarget drops ~50% of the threat of main target -> next iteration. Looks like we agree to disagree. Therefore the only logical solution is to have 2 different versions. Test them properly. Only then (maybe) we will come to better conclusions, understanding of the good, the bad and the ugly about every way. Gotisch : thanks for summarizing it all up. It is pretty accurate and will surely help people too lazy following the walls of text from above EDIT: didn't want to make new post. below patch adds support for extractor under linux. CMakeLists.txt : http://pastie.org/1223272 code diff against 213e909001 :: http://pastie.org/1223273 cmake file can be used to generate any project under VC as well. The source changes include mainly missing includes, warning fixes. The only "real" issue was that *nix systems are case sensitive while win aren't. Take care.

That's the main problem. If we could check for reachibility "for free", there would not be any problem "just checking" before selecting target. What you actually saying is same thing faramir118 does : all paths at once. Sorry, I had to edit my previous post with some thoughts on the subject.

iteration start (update call): - MovementGenerator:: Generate path to current target. - SelectHostileTarget:: Path generated for current target is valid -> noop. else mark current target in thread list as unreachable select target with highest thread (and unmarked) from thread list (+ all current readability tests) Iteration end. If we have 0 valid (path wise valid) targets, out of X, it will take us X updates to figured that out. By all means, it is finite, best effort. If here is valid reachable target, it will find it. By the time SelectHostileTarget runs it has all the data it needs to make educated decision. MovementGenerator don't have to know anything - it takes its commands from AI. Exactly the same applies to 2nd solution too. In most cases first target we select will also be the last. Probably 99% of units which call SelectHostileTarget use TargetedMovementGenerator: SelectHostileTarget calls AI()->AttackStart(). All core AI implementations of AttackStart() call MoveChase. I'm not familiar enough with scriptdev2's AI implementations to know what to do to allow them to skip pathfinding. I'll post more on this after I look into it. Sure, its just Players derive from Unit, Units not in combat do not chase anything. So it still stands : "maybe 1% of all units in any given moment". Maybe I'm really failing to see the major advantages of generating paths as soon as SelectHostileTarget over generating them in movement generators. Please do explain me. I'll try to explain once again why I'm against generating paths in AI code. Firstly and most importantly : it single handedly destroys entire modularity concept. By creating those ties between unrelated section of the code, unnecessary dependencies created. Integrating them one into another just creates blob of code. AI: what to do. MovementGen: how to do. We should not mix those. 2nd: path generated are valid until ANY target moves. If you generate X paths every update call, you are wasting alot of resources here for very, very little gain ( corner case ). We may not want to solve it at all. Consider it : as soon as primary target changes, the creature will switch target anyhow. I think something like can be fair solution: if cannot reach main target, try one more target, if not evade. The more I think about it, the faster I come to conclusion that we should not deal with this case at all. All that has to be done, is to move the evading code out from movement generator to SelectHostileTarget and "call it a day". If main target not reachable due to terrain - evade. This cannot be "natural" case. Consider this : all thread list out of reach ( since they found hold in map and they are abusing it or something ). How is that different from main tank constantly out of reach? I think it isn't. Back to my point : We should just evade. End of story. Thanks in advance.

I see no problem calling all those while we move in direction of something we consider "non-optimal" target. All the code in-between (1) and (2) should run anyway. If the creature dies, or whatever, we wont have to continue anyhow, already set evade flag wont change anything. Maybe the unit can cast spells while on its way, who knows ... No, By the time it gets to SelectHostileTarget, MovementGenerator already made path. There's no reason generating anything new here. This is easy, just store path in Unit instead of MovementGenerator. That's right, "design-oriented viewpoint" - MovementGenerator deals with the movement, AI deals with the aggro. That what I was saying all along. That's why I'm against generating paths in SelectHostileTarget. We can check the type of already generated path inthere, but not dealing with path logic. Thats why we cannot store paths in Unit too. In addition to the fact it is wont be used by anything but npc's which use targetmovement generator ( its maybe 1% of all units in any given moment). I don't know how often this will actually happen... In most cases, players are solo and monsters will just reset (or attack the pet if there is one). In raids, players are always right in the fight. Another reason why we shouldn't add any heavy code dealing with this corner case. EDIT: I took another look at the pseudocode you added for my suggestion, and it isn't exactly what I had in mind. All 3 stars can be replaced with "select different feasible target" - no path generating involved there. Just checking what type the current path has. And (1) can/should be removed ( after (*) added ). PS: sorry for such late edit.

After thinking about it, I think it should stay the way it is in current mangos. First, I will try to explain why I think, your idea wont work well ( atleast under current layout ). We generate path under MovementGenerator. From what I understand, if we change the scheme to what you suggest, we will have to either generate paths as soon as isInAccessiblePlaceFor, and then somehow copying them to movement master, or generating those path more than once. This is totally unacceptable performance wise. If we keep the current scheme, after we generate the path in (1), we get to (2) - there we can check the type of the path. If we can select better target, set new target for next MovementGenerator update. We do "skip" one iteration here, but thats totally expendable when you compare it with generating extra paths ( checking reachability ). Eventually we will select "the best target", it may take few extra update calls, but it will take overall less path calls. Keep in mind that if we will try to calculate paths to all feasible targets on same update we're creating bottleneck. Spreading those calls is much better choice. I think we should only modify Unit::SelectHostileTarget() to check current path type - if it isn't complete => try to select different target. No targets at all => evade ( after they came as close as possible etc .. ). Just my thoughts, Ideas are welcomed. The more time we spend on planning, the less time we'll spend dealing with problems. About the temp evade solution I have in my patch : for now its better than nothing. Can always be removed when better implemented. It has nothing to do with actual pathfinding code, it just prevents loops when target is not reachable. I still think it should be added for now.

You can run current under wine just fine, or just extract under win machine and transfer.