Jump to content

Attack notification


Recommended Posts

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 by madmax
Link to comment
Share on other sites

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

Link to comment
Share on other sites

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
#

Link to comment
Share on other sites

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 by madmax
Link to comment
Share on other sites

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

Link to comment
Share on other sites

I take that back, it does seem to work now :unknw: 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 by zoot
Link to comment
Share on other sites

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 by madmax
Link to comment
Share on other sites

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 by zoot
Link to comment
Share on other sites

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)
$

Link to comment
Share on other sites

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.)

Link to comment
Share on other sites

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.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

×
×
  • Create New...