- User Since
- Apr 5 2019, 7:29 PM (15 w, 7 h)
Doesn't this close D843 also?
This revision now requires review to proceed.
Not sure why and how I triggered that.
- return true after execute.
That is interesting, for me it does work (at least yesterday it did). Do you get an error or something?
Thu, Jul 18
Wait for result of D2070 discussion. Especially regarding curly braces.
If restricted classes would be also applicable to damage (also splash/death/trample) immunity would not be necessary, I guess.
This merge-conflicts a bunch with D1938
Not sure where it conflicts with D1938, yes some rebasing is needed when either is committed but no major changes needed as far as I can see.
Thanks for committing this @bb!
Wed, Jul 17
I've been testing this, UM looks so much better now than a few weeks ago! Kudos @wraitii.
Regarding this specific diff, no strange things found.
Perhaps you can add a reference to the source? For future reference and may help to persuade people ;)
- Proper checking wheter an entity is moving.
Tue, Jul 16
I agree with @wraitii that using capture as a damage type is weird ;) But also agree that a modder should have the option to do both damage and capture at the same time.
The armour for capture is interesting, but isn't there the CP-regenrate for already?
Wouldn't beams be just ranged attack with a stream of projectiles? Or am I thinking to limited now.
Make Jenkins/Vulcan a tad happier.
Mon, Jul 15
Well, the function GetPhenotype *is* called for them (i.e. mines, trees, buildings etc. (not fish; don't ask me why)), so I'm not sure that "default" would not throw errors, but I'll test.
I wonder if all this shouldn't be handled in cmpidentity. To make the other components agnostic. That would simplify the code a lot here and in cmpvisual actor I think you wanted to do it
CauseAreaDamage => CauseDamageOverArea.
- Moved cmpDamage-function to CauseSplashDamage.
- Removed support for status effects and restrictedClasses which are to be added in a seperete diff (D2081)
I can't get the test working properly, too little experience with that I guess, but I do not really want to leave it out. I'll try harder soon.
Sun, Jul 14
The armour is used nowhere in tests, should a new test (test_armour) be added?
From what I read rP20625 included the variable g_DamageTypes to the wall_builder-script because getting the template required that.
Regarding rP20203, this seems indeed an extension of that commit. If I read it correctly, it was desired to have it more generic, but no one did it yet.
Hi! I got a segfault when testing this:
- One var => let.
and always discourage non-core members from doing wholesale commits like this for obvious reasons..
Sorry, didn't know,,,
I did not really change any code (except one var -> let) and, as far as I can tell, you are a very exact person. So yeah, I think you are ;)
Thanks for the info! It might be in scope to change it then, but I guess that needs a team decision.
Sat, Jul 13
Reuploading because the test doesn't fail locally.
Some more fixes (last?).
Well, I think that the term "splash" is still correct in many places. It is splash damage that a projectile causes, right? You're more proficient with language ;)
I agree though that regarding DeathDamage it should probably be replaced.
- Added tooltip.
- Many fixes from inlines.
Test not yet done.
Thu, Jul 11
Some more checks in PetraAI.
I think what he meant is that when an entity kills its direct opponent it will cheer, but it might be the case that while cheering (s)he is under arrow fire from afar.
Wed, Jul 10
Whilst testing this I got:
*** stack smashing detected ***: <unknown> terminated
Not going to work on this any time soon.
I'll wait untill you find an agreement ;)
Improved tooltip to also show extra garrison size.
- slots -> size
- Extended test.
- Good check.
- Also disables trade window when there are neither resources to trade nor to barter.
If everything went okay now, PetraAI can handle situations where there are no resources tradable, tributable and/or barterable.
Yeah, but there is happening a lot before it loops through the resources ;)
Even more fixes.
I tested these situations just now:
- No Bartering resources reveals no errors,
- No Trading resources gives only errors when trying to trade (obviously).
- No Tributing resources gives an error when opening the diplomacy screen (which I think can easily be fixed by setting a default for widthOffset).
Performance-wise it would be good if PetraAI would not even try to initialise the different components when they are not possible.
Question: Should we also try to account for the cases that no resources have a specific property?
Tue, Jul 9
Should do the job, @Angen.
Many small fixes.
You beat me to it @Angen ;)
So glad you're testing it already! I'm starting to get a love-hate relationship with PetraAI ;) (The implementation of the garrison slots took me about half an hour, fixing Petra took the rest of the day,,,)
Duplicated in D1268. Sorry people.
- Some fixes regarding PetraAI.
Thanks! I'm currently running many test games AI vs AI and already found lots of errors :( Mainly when I use *.id() where it should not be used and vice versa.
So here is a problem: I cannot find any code to give a (bombard) circle overlay at the pointer position. (Range visualisation is a component of an entity.) Currently a tooltip is shown with the bombard radius, but that seems kind of odd. Moreover, distances/radii are quite difficult to estimate in-game, IMHO.
Hence the question what to do with it now?
- Nothing extra, we can just use it as is. (Not likely because of reasons above.)
- Split the radius-changing part as requested earlier. (Probably, but I'm no fan of that for reasons mentioned earlier (here & here ).)
- There *is* a way of showing the radius! (Please point me in that direction then ;) )
- We will make a way of showing the radius. (My last encounter with C++ was not very comfortable, but I still am willing to learn.)
Or something else?
Mon, Jul 8
- Improved function canAttackTarget in entity.js in common-api.
- More checks whether the AI can attack units.
Also, what to do with attackMove?
Sun, Jul 7
Created a canAttackTarget-function in "entity.js" of the "common-api".
I'm not sure, but is seems that I cannot check the target there, is that correct? target.anyFunction() gives an error.
Shouldn't the check be in entity.js of the common-api? There is also a function canAttackClass already in that file.
Can I somehow use the function CanAttack from Attack.js for that?
Added unit test.