Not self explanatory code in bombsite location snippet

[skirsten]

The following code

var relevantTrigger = parser.triggers.Single(a => a.Index == site);

if ((parser.bombsiteACenter - bombEventArgs.Player.Position).Absolute <
    (parser.bombsiteBCenter - bombEventArgs.Player.Position).Absolute) {
    // planted at A.

    bombEventArgs.Site = 'A';
    parser.bombsiteAIndex = site;
} else if (relevantTrigger.Contains(parser.bombsiteBCenter)) {
    // planted at B.

    bombEventArgs.Site = 'B';
    parser.bombsiteBIndex = site;
}

GameEventHandler.cs Line 303

raises multiple questions:

1. Why the bounding box check at bombsite b? Is this not sufficient enough:

   if ((parser.bombsiteACenter - bombEventArgs.Player.Position).Absolute <
       (parser.bombsiteBCenter - bombEventArgs.Player.Position).Absolute) {
       // planted at A.
       // set site...
   } else {
       // planted at B.
       // set site...
   }

2. Why not use only the bounding box:

   var relevantTrigger = parser.triggers.Single(a => a.Index == site);
   
   if (relevantTrigger.Contains(parser.bombsiteACenter)) {
       // planted at A.
       // set site...
   } else {
       // planted at B.
       // set site...
   }

Am I missing something?

Also: Which of the used methods (absolute relative position or bounding box) should be used to identify the bombsite in the BombBeginDefuse and BombAbortDefuse events? (They do not contain site information)

Or should I save the last planted site and use that?

Anyway thanks for the awesome project!

[moritzuehling]

You're right, thanks!

We used only the bounding-box before, this sadly gave some errors on some maps. However, the changed code didn't really fix this, because of the sad error.

You can create a pull-request with any of those fixes, I'm fine with either.

[skirsten]

#89 changed bombsite detection code

[moritzuehling]

Fixed in #89