Attack notification

madmax · July 5, 2013

Zoot,

I have pushed the latest changes. Once you confirm, I ll send out a patch.

Blinking red is confusing with red being a typical player color, white might work better (and some other rendering enhancements in the future) - done
The interface of ICmpMinimap::GetRemainingPingCount()/SetRemainingPingCount() is awkward and only used in one place, could that be replaced by a single Ping() function that checks and decrements the counter? - done
More importantly, pinging is tied into the framerate, so at high framerates it will blink too fast, on low framerates it will appear to not blink at all, and also it will blink for a much shorter time at high framerates and much longer at low framerates. I think it should use time instead and measure time between minimap draw calls.
-done
Can't the "attacked" sound group be added to the parent template_unit.xml instead of all the inheriting templates?
- done
CCmpMinimap::m_PingEntity is a confusing name because it's type bool not entity_id_t that might be expected. Could it be named m_IsPinging instead? Similarly, IsEntityPinging() could be simplified to IsPinging(), since it's a component it will of course be an entity - done
Speaking of configuration, it occurred to me that adjusting how long the attack notifications ping on the minimap would be a useful option, and that is certainly possible (in C++). I believe the function to do that is CFG_GET_VAL. - done
Braces should go on line by themselves, see lines 476,483 in MiniMap.cpp - fixed
Something else I thought about, CCmpMinimap should serialize its pinging state. That way saving and reloading won't lose active notifications. I forgot to test this when I had the patch applied, but looking at the code it seems that is missing. Can you confirm? Try quicksave/quickload (Shift+F5, Shift+F8) - Serialized ping state

https://github.com/laxmax/0ad/commits/attack-notification

Edited July 5, 2013 by madmax

zoot · July 5, 2013

I'm getting merge errors when I do "git pull 0ad master". Am I doing something wrong or do you get that too?

zoot · July 5, 2013

Seems someone (historic_bruno) has made modifications to MiniMap.cpp while you were away. You'll need to merge these changes into your own branch such that your code fits the layout historic_bruno has introduced. (I would do it, but I don't know the proper semantics of the minimap code.)

http://git-scm.com/book/ch3-2.html#Basic-Merge-Conflicts

madmax · July 6, 2013

ok, there also seems to be a conflict in Armour.js Is that resolved ? How do I resolve it, now that I have pulled from the master :


abhi@ABHI-PC /f/Code/0ad/0ad (attack-notification)
$ git pull k776-master master
remote: Counting objects: 2669, done.
remote: Compressing objects: 100% (1035/1035), done.
remote: Total 2041 (delta 1370), reused 1658 (delta 995)
Receiving objects: 100% (2041/2041), 12.79 MiB | 306 KiB/s, done.
Resolving deltas: 100% (1370/1370), completed with 337 local objects.
From https://github.com/0ad/0ad
* branch		    master	 -> FETCH_HEAD
Performing inexact rename detection: 100% (18204/18204), done.
Auto-merging source/soundmanager/scripting/SoundGroup.h
Auto-merging source/soundmanager/scripting/SoundGroup.cpp
Removing source/soundmanager/js/SoundPlayer.h
Removing source/soundmanager/js/SoundPlayer.cpp
Removing source/soundmanager/js/Sound.h
Removing source/soundmanager/js/Sound.cpp
Removing source/soundmanager/js/MusicSound.h
Removing source/soundmanager/js/MusicSound.cpp
Removing source/soundmanager/js/MusicList.h
Removing source/soundmanager/js/MusicList.cpp
Removing source/soundmanager/js/AmbientSound.h
Removing source/soundmanager/js/AmbientSound.cpp
Auto-merging source/gui/MiniMap.h
Auto-merging source/gui/MiniMap.cpp
CONFLICT (content): Merge conflict in source/gui/MiniMap.cpp
Auto-merging binaries/data/mods/public/simulation/templates/template_unit_infantry.xml
Auto-merging binaries/data/mods/public/simulation/templates/template_unit_champion.xml
Auto-merging binaries/data/mods/public/simulation/templates/template_unit_cavalry.xml
Auto-merging binaries/data/mods/public/simulation/templates/template_unit.xml
Auto-merging binaries/data/mods/public/simulation/templates/template_structure_economic_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/template_structure.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/spart_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/rome_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/pers_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/maur_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/mace_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/iber_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/hele_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/gaul_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/celt_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/cart_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/brit_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/structures/athen_storehouse.xml
Auto-merging binaries/data/mods/public/simulation/templates/special/player.xml
Auto-merging binaries/data/mods/public/simulation/components/Attack.js
Auto-merging binaries/data/mods/public/simulation/components/Armour.js
CONFLICT (content): Merge conflict in binaries/data/mods/public/simulation/components/Armour.js
Removing binaries/data/mods/public/gui/text/tips/mills.txt
Auto-merging binaries/data/mods/public/gui/session/messages.js
Auto-merging binaries/data/mods/public/audio/interface/select/building/sel_storehouse.xml
Removing binaries/data/mods/public/audio/interface/select/building/sel_mill.ogg
Auto-merging binaries/data/mods/public/audio/interface/complete/building/complete_storehouse.xml
Removing binaries/data/mods/public/audio/interface/complete/building/complete_mill.ogg
Removing binaries/data/mods/public/art/textures/skins/props/shield/mace_silver_c.dds
Removing binaries/data/mods/public/art/textures/skins/props/shield/mace_silver_b.dds
Removing binaries/data/mods/public/art/textures/skins/props/shield/mace_silver_a.dds
Auto-merging binaries/data/mods/public/art/actors/structures/romans/storehouse.xml
Auto-merging binaries/data/mods/public/art/actors/structures/persians/storehouse.xml
Auto-merging binaries/data/mods/public/art/actors/structures/mauryans/storehouse.xml
Auto-merging binaries/data/mods/public/art/actors/structures/iberians/storehouse.xml
Removing binaries/data/mods/public/art/actors/structures/hellenes/mill.xml
Auto-merging binaries/data/mods/public/art/actors/structures/celts/storehouse.xml
Auto-merging binaries/data/mods/public/art/actors/structures/carthaginians/storehouse.xml
Removing binaries/data/mods/public/art/actors/structures/athenians/mill.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/romans/storehouse.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/persians/storehouse.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/mauryans/storehouse_props.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/iberians/storehouse.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/hellenes/storehouse_wood.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/hellenes/storehouse_roof.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/hellenes/storehouse_props.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/hellenes/storehouse_blocks.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/celts/storehouse_shield.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/celts/storehouse_2.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/celts/storehouse_1.xml
Auto-merging binaries/data/mods/public/art/actors/props/structures/carthaginians/storehouse.xml
Auto-merging binaries/data/config/default.cfg
Automatic merge failed; fix conflicts and then commit the result.
abhi@ABHI-PC /f/Code/0ad/0ad (attack-notification|MERGING)
$


# Unmerged paths:
#   (use "git add <file>..." to mark resolution)
#
#	   both modified:	  binaries/data/mods/public/simulation/components/Armour.js
#	   both modified:	  source/gui/MiniMap.cpp
#

madmax · July 6, 2013

ok, its just a comment, I am accepting it :


Armour.prototype.SetInvulnerability = function(invulnerability)
{
this.invulnerable = invulnerability;
};
<<<<<<< HEAD
Armour.prototype.TakeDamage = function(hack, pierce, crush, source)
=======
/**
* Take damage according to the entity's armor.
* Returns object of the form { "killed": false, "change": -12 }
*/
Armour.prototype.TakeDamage = function(hack, pierce, crush)
>>>>>>> 5e04aea6bbce9aba0accf2b411acb17621df8cf4
{

I have made it like this :


Armour.prototype.SetInvulnerability = function(invulnerability)
{
this.invulnerable = invulnerability;
};
/**
* Take damage according to the entity's armor.
* Returns object of the form { "killed": false, "change": -12 }
*/
Armour.prototype.TakeDamage = function(hack, pierce, crush)
{
// Alert target owner of attack
var cmpOwnership = Engine.QueryInterface(this.entity, IID_Ownership);
var cmpAttackDetection = QueryPlayerIDInterface(cmpOwnership.GetOwner(), IID_AttackDetection);
var cmpTimer = Engine.QueryInterface(SYSTEM_ENTITY, IID_Timer);

OK, I pushed my changes to the 2 files. I am still getting a large number of Javascript errors.

interestinglog.html

Edited July 6, 2013 by madmax

zoot · July 6, 2013

I believe you also need to add the 'source' argument back on:

Armour.prototype.TakeDamage = function(hack, pierce, crush, source)

zoot · July 6, 2013

Also, have you recompiled? I believe this error should be fixed in latest 0ad/master:

ERROR: CXeromyces: Parse error: gaia/flora_tree_carob:1: Element VisualActor has extra content: VisibleInAtlasOnly

zoot · July 6, 2013

I have the above suggested changes in my branch:

https://github.com/z...ck-notification

It compiles and I don't get any errors when running it, but unfortunately the minimap blinking does not seem to work. Perhaps you made a mistake in the merge?

Edited July 6, 2013 by zoot

zoot · July 6, 2013

It compiles and I don't get any errors when running it, but unfortunately the minimap blinking does not seem to work. Perhaps you made a mistake in the merge?

I take that back, it does seem to work now :unknw: I must have failed to compile properly the first time I tried.

If it works fine for you, I consider the latest commit on my branch good to go: https://github.com/zootzoot/0ad/tree/attack-notification

zoot · July 6, 2013

I take that back, it does seem to work now I must have failed to compile properly the first time I tried.

Or maybe what sometimes happens is that other, non-blinking dots are drawn on top of the ones that do blink. Unless there is an easy fix for that we can probably ignore it for now.

Edited July 6, 2013 by zoot

madmax · July 6, 2013

ok, I am still getting js errors :(, should I pull from k776-master

It was created as follows :

git remote add k776-master https://github.com/0ad/0ad.git

So I run this perhaps :

git pull k776-master master

I already pulled from :

git pull zootzoot attack-notification

It was created as :

git remote add zootzoot https://github.com/zootzoot/0ad.git

I see the 'source' argument added in Armour.js. I though if I pull from : git pull zootzoot attack-notification

that will include the master changes too ? But maybe it does not.

Yeah for that the blinking dots have to be drawn last. Probably they can be collected in a new vertex array and sent to the gpu after the current vertices which are for non-blinking players.

It would be good to have some input on this graphical problem from someone more familiar with minimap drawing. Do I add another VertexArray member in Minimap.h & use it ? This has to be done in the most efficient possible way for the GPU.

interestinglog.html

Edited July 6, 2013 by madmax

zoot · July 6, 2013

No, you should only need to pull from zootzoot. If everything else fails, you could do:

git reset --hard ede7f185cded23542dde6fc01689912d2dd5b7d9
git pull zootzoot attack-notification

Note that this will delete any commits you may have made since this commit. But it should make things work. (It does for me.)

Edited July 6, 2013 by zoot

madmax · July 6, 2013

ok, I ll back up all the files and do the reset.

zoot · July 6, 2013

Are you completely positive that you have updated workspaces and recompiled? The JS error you get should not happen in the latest versions of the executable.

madmax · July 6, 2013

ok, I am trying a clean recompile now, lets hope it fixes it.

This is what happened after I updated last :


abhi@ABHI-PC /f/Code/0ad/0ad (attack-notification)
$ git log --branches --not --remotes --simplify-by-decoration --decorate --oneline
abhi@ABHI-PC /f/Code/0ad/0ad (attack-notification)
$ git pull zootzoot attack-notification
remote: Counting objects: 33, done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 20 (delta 14), reused 17 (delta 11)
Unpacking objects: 100% (20/20), done.
From https://github.com/zootzoot/0ad
* branch		    attack-notification -> FETCH_HEAD
Updating ede7f18..7fffc7f
Fast-forward
binaries/data/config/default.cfg						  |  1 +
binaries/data/mods/public/gui/session/messages.js		 | 13 ++++++-------
binaries/data/mods/public/simulation/components/Armour.js |  2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
abhi@ABHI-PC /f/Code/0ad/0ad (attack-notification)
$ git diff
abhi@ABHI-PC /f/Code/0ad/0ad (attack-notification)
$

zoot · July 6, 2013

This is what happened after I updated last :

Looks correct.

madmax · July 6, 2013

Nope doesnt fix it, I ll have to do a hard reset.

madmax · July 6, 2013

Hard reset doesnt work either. Is there an even more drastic solution ? Can I clear out the directory and have git automatically restore all files ?

Edited July 6, 2013 by madmax

zoot · July 6, 2013

Wow I believe git clean removes files that may not be tracked for git reset.

zoot · July 6, 2013

The most drastic possible step would be to delete your root 0 A.D. Git directory and clone it anew from my repo.

madmax · July 6, 2013

ok and this delete of the root 0ad git directory will not be registered with git in someway I guess as I will be deleting it from windows.

zoot · July 6, 2013

ok and this delete of the root 0ad git directory will not be registered with git in someway I guess as I will be deleting it from windows.

No, everything, including Git's "knowledge", is stored below the root Git directory. (In .git subdirectories.)

zoot · July 6, 2013

I wonder what could have happened that would require such a drastic step, though. Hopefully the errors you get are not due to some odd, completely unrelated bug in the engine.

Edited July 6, 2013 by zoot

madmax · July 6, 2013

arrgghh!!!, human stupidity , not a engine bug. I was compiling in debug mode in visual studio but running pyrogenesis.exe (I should have been running pyrogenenesis_dbg.exe)

Now I see that the blinking stops very quickly, will try to fix it.

zoot · July 6, 2013

(y)

I'll be AFK for a bit, but will be checking by phone every now and then.

Attack notification

Recommended Posts

Link to comment

Share on other sites

Top Posters In This Topic

Popular Days

Top Posters In This Topic

Popular Days

Popular Posts

zoot

Mythos_Ruler

zoot

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation