Attack notification

[zoot]

Yes, definitely strive to add them to parent templates only. Otherwise you will waste a lot of your own time as well as make the templates harder to maintain.

[madmax]

Ok thanks, will do.

There is 1 thing I noticed with 2 Ashoka units on the map, that the blinking continues even if the unit is not under attack anymore. Thats because I have made it blink for a certain number of frames before shutting down. So if the frame rate is high it will probably stop sooner. That can be fixed by using the number of game turns instead.

I wonder though, if the units should keep on blinking after the player has moved it away from peril. Then we would need a attack stopped notification from the JS sim code. Guess its not worth it.

Edited April 4, 2013 by madmax

[zoot]

Suggestion: What do you think about moving the ping functionality into the CCmpMinimap.cpp component (much like you originally suggested), but then instead of triggering it from messages.js once, together with the sound, it would be triggered directly by the AttackDetection.js component every time a unit is hit?

That way units still under attack would keep blinking even after the one initially pinged has died, and it would make more units in a skirmish blink, thus making it easier to see.

Edited April 4, 2013 by zoot

[madmax]

Ok, is this because you see some units are being attacked but not being pinged in the minimap ? Because the logic for playing the sound suppresses some of these notifications to prevent the sound/attack notifications being constantly played ?

[zoot]

Ok, is this because you see some units are being attacked but not being pinged in the minimap ? Because the logic for playing the sound suppresses some of these notifications to prevent the sound/attack notifications being constantly played ?

Yes, basically. We should still be able to see that units are under attack, even if the initially pinged fellow died.

[madmax]

Right, yeah I ll work on it today. Can you tell me where in Attack.js you will be putting the PingMinimap calls so I can put them there and test after I have moved the PingMinimap function to the ICmpMinimap interface.

So just to clear it in my big confused head, we are doing this :

but then instead of triggering it from messages.js once, together with the sound, it would be triggered directly by the AttackDetection.js component every time a unit is hit

because AttackDetection.js needs to notify the minimap much more frequently than the sound needs to be played. So if an unit has died, other units in the area may still be under attack but we choose to no longer emit a sound as the user's attention has (hopefully!) already been drawn to the battle.

But we choose to notify the user on the minimap about these other units as all units under attack should be pinging in the minimap. This is to ensure that the user does not think that the battle has ended and go back to tending the economy !

Also can't AttackDetection.js call the currently exposed global PingMinimap function. If I make the change will I be able to revert to the current situation of the files later using the commit hash code ?

Edited April 5, 2013 by madmax

[zoot]

Yes, you can always go back to the earlier commit. See e.g.:

http://ariejan.net/2011/09/08/git-remove-reset-and-rollback-commits/

http://davsblog.me/how-to-do-rollbacks-in-git-in-other-words-oop

Another option would be to create a new branch from your existing one (Git branches are designed specifically to be cheap and light). If your work in the new branch works out, you can merge it back into the old one. If it doesn't, you simply switch back to the old one.

http://git-scm.com/book/en/Git-Branching-Basic-Branching-and-Merging

[zoot]

Here is what I have now, you'll decide whether it seems workable: https://github.com/zootzoot/0ad/commit/1d5db6f8d4511915266d859c25950b98fc72e238

Instead of calling Engine.PingMinimap(), this posts a message of type MT_EntityAttacked to the attacked entity. You should be able to intercept this message in the CCmpMinimap.cpp component, like it already intercepts MT_PositionChanged and MT_OwnershipChanged messages: https://github.com/0ad/0ad/blob/master/source/simulation2/components/CCmpMinimap.cpp#L139

The message type would have to be defined on the C++ side first, though.

[madmax]

ok, yeah that makes sense. Then there is no need for extra code to expose yet another global function for a functionality that isnt really global but specific to an entity.

2 questions here :

First, my local repo was cloned from your repo, so a git checkout attack-notification should suffice to pull in your latest JS changes ? Or rather a

git pull I guess

Secondly.I ll need to figure out how to access the minimap object from CCmpMinimap. Need to look at the code in other components to see if they access GUI objects using CGuiManager or something similar

Edited April 5, 2013 by madmax

[zoot]

To pull my changes into the current branch in your own repo, you would do something along the lines of:

git remote add zootzoot https://github.com/zootzoot/0ad.git
git pull zootzoot/attack-notification

The first line adds my repo as a remote, and the second one does the pull.

Secondly.I ll need to figure out how to access the minimap object from CCmpMinimap. Need to look at the code in other components to see if they access GUI objects using CGuiManager or something similar

There must be some kind of interface between CCmpMinimap and the actual minimap already, but yeah, I don't know if it suffices for this task.

[madmax]

abhi@ABHI-PC /f/Code/0ad/0ad (attack-notification)
$ git remote add zootzoot https://github.com/zootzoot/0ad.git

abhi@ABHI-PC /f/Code/0ad/0ad (attack-notification)
$ git pull zootzoot/attack-notification
fatal: 'zootzoot/attack-notification' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

abhi@ABHI-PC /f/Code/0ad/0ad (attack-notification)

Unable to pull in your changes

----------

ok got it, no slash there

Edited April 5, 2013 by madmax

[zoot]

Right, my bad

[madmax]

I changed the order of the args in the event to make it consistent with the remaning messages, entity id first :

// Message of the form { "event": { "target": 123, "position": { "x": 123, "z": 456 }, "time": 1 }.
// sent when an entity is attacked.
Engine.RegisterMessageType("EntityAttacked");
// Message of the form { "player": 1, "event": { "target": 123 , "position": { "x": 123, "z": 456 }, "time": 1, }.
// sent when a new attack is detected.
Engine.RegisterMessageType("AttackDetected");

and line 46 in AttackDetection.js :

var event = {target: target, position: cmpPosition.GetPosition(), time: cmpTimer.GetTime()};

Would any other code be affected. What about consumers of the message MT_AttackDetected. This message will not be declared or used in C++ (unless we plan to , not sure)

Edited April 5, 2013 by madmax

[zoot]

None that I can think of. There are no consumers of MT_AttackDetected yet. If there is any affected code, we should find out soon enough.

[madmax]

I realized since you are sending the message only to the target entity and the message will get registered in that entity's CmpMinimap component, you do not need to pass the entity id at all. But let it be around anyway.

Its like 1 entity has multiple components.

Edited April 5, 2013 by madmax

[zoot]

Its like 1 entity has multiple components.

Hmm. What are you referring to? Entities do indeed have multiple components. But hopefully only one minimap component?

Edited April 5, 2013 by zoot

[madmax]

Yes it has only 1 component of a particular type, so just 1 Minimap component.

Ok so I have done all the C++ changes, but I still get a JS error even though the game plays fine. You can pull my latest commit an check.

https://github.com/laxmax/0ad/commit/b1e5e5b2cee620bbdce74afc06b4ab04ee3a3479

Seems the following is the location of the problem:

var cmpTimer = Engine.QueryInterface(SYSTEM_ENTITY, IID_Timer);
var event = {target: target, position: cmpPosition.GetPosition(), time: cmpTimer.GetTime()};

Engine.PostMessage(target, MT_EntityAttacked, event);

position: cmpPosition.GetPosition() ...maybe it should be received in a var called pos and then split up

var event = {target: target, pos.x, pos.z, time: cmpTimer.GetTime()};

Edited April 5, 2013 by madmax

[zoot]

Seems there may be a mismatch between your definition in MessageTypes.h and the format of the 'event' variable. If you can't make the definition in MessageTypes.h match, you should just leave the event variable out of the call (since it's used elsewhere and needs to retain its format) and pass a literal, like:

Engine.PostMessage(target, MT_EntityAttacked, {target: target, posx: pos.x, posz: pos.z, time: cmpTimer.GetTime()});

I haven't used MessageTypes.h before, so I am not completely sure what is expected there, but hopefully something in that direction.

[zoot]

Hmm. The docs says you need to add a "COMPONENT" line to TypeList.h. Without that, the error could be due to the "MT_EntityAttacked" message type constant not being properly registered on the JS side.

[zoot]

Hmm. The docs says you need to add a "COMPONENT" line to TypeList.h. Without that, the error could be due to the "MT_EntityAttacked" message type constant not being properly registered on the JS side.

Actually, no, that's for component interfaces, not messages

You seem to have added the message to MessageTypeConversions.cpp properly.

Edited April 5, 2013 by zoot

[zoot]

The error went away when I did this: https://github.com/zootzoot/0ad/commit/1a1641487a8d1570d93a45ee70c1d30296801e84

(If you pull you may have to recompile, since I've also merged in the latest 0ad/master.)

[madmax]

Thanks, so I have committed the change which allows pinging to appear in the minimap via the interface and message thing.

https://github.com/z...5950b98fc72e238

The minimap interface implements only dumb pinging. It has no notion of why the pinging occurs as it can be for any reason. We may want pinging to happen to indicate other things later on. Also it does not do any check on the user's player ID. The JS sim code currently sends messages for attack on any entity. I think what the JS code should do is send messages only if entities of the current user is being attacked. Right now any attacked entity, even those of the enemy is being pinged. It was working correctly with the earlier JS where you were using the global PingMinimap()

Also currently the pinging will stop if the unit is no longer attacked, after a duration decided by the user's frame rate. Which is weird but I guess not an issue as it doesnt affect the sim, only notification to the user. Trying to use the number of turns adds more code to no real purpose.

Edited April 6, 2013 by madmax

[zoot]

Also it does not do any check on the user's player ID. The JS sim code currently sends messages for attack on any entity. I think what the JS code should do is send messages only if entities of the current user is being attacked. Right now any attacked entity, even those of the enemy is being pinged. It was working correctly with the earlier JS where you were using the global PingMinimap().

The sim has no concept of who the current player is, any such check has to be done on the GUI side.

Edited April 6, 2013 by zoot

[madmax]

Oh ok, I can try to implement the logic. But the call to the global PingMinimap() was sending only for the current player. I wonder how. Maybe because it was at global scope and it can detect the player.

Edited April 6, 2013 by madmax

[zoot]

The PingMinimap call was sent from messages.js where a check on the player is done. (messages.js is in the GUI.)