search

secondlife / jira-archive

2 stars 0 forks source link

[BUG-37711] A fix for the damage icron not being displayed properly. #12037

Open sl-service-account opened 8 years ago

sl-service-account commented 8 years ago

Steps to Reproduce

—

Actual Behavior

For some time, I just thought the damage icon was never display because it was removed, but it turns out it's due to a check for two flags being enabled, instead of a check for either flag being enabled.

I have to make a new one, because for some dumb reason I cannot comment or mention anything on any previous bug post, and this bothers me greatly.

I spent a moment looking in, and after making the following change:

llviewerparcelmgr.cpp : line 706


bool LLViewerParcelMgr::allowAgentDamage(const LLViewerRegion* region, const LLParcel* parcel) const
{
    return (region && region->getAllowDamage())
        && (parcel && parcel->getAllowDamage());
}

SHOULD BE


bool LLViewerParcelMgr::allowAgentDamage(const LLViewerRegion* region, const LLParcel* parcel) const
{
    return (region && region->getAllowDamage())
        || (parcel && parcel->getAllowDamage());
}

It has completely resolved the issue for me, and avatar health indicators show up fine now.

Expected Behavior

—

Other information

BUG-9097 is locked down, so I'll mention this is a fix for that.

Links

Related

Original Jira Fields | Field | Value | | ------------- | ------------- | | Issue | BUG-37711 | | Summary | A fix for the damage icron not being displayed properly. | | Type | Bug | | Priority | Unset | | Status | Accepted | | Resolution | Accepted | | Reporter | Moy Loon (moy.loon) | | Created at | 2016-08-21T03:34:18Z | | Updated at | 2016-08-22T17:15:22Z | ``` { 'Business Unit': ['Platform'], 'Date of First Response': '2016-08-20T22:38:03.656-0500', "Is there anything you'd like to add?": "BUG-9097 is locked down, so I'll mention this is a fix for that.", 'ReOpened Count': 0.0, 'Severity': 'Unset', 'System': 'SL Viewer', 'Target Viewer Version': 'viewer-development', 'What just happened?': "For some time, I just thought the damage icon was never display because it was removed, but it turns out it's due to a check for two flags being enabled, instead of a check for *either* flag being enabled.\r\n\r\nI have to make a new one, because for some dumb reason I cannot comment or mention anything on any previous bug post, and this bothers me greatly.\r\n\r\nI spent a moment looking in, and after making the following change:\r\n\r\nllviewerparcelmgr.cpp : line 706\r\n\r\nbool LLViewerParcelMgr::allowAgentDamage(const LLViewerRegion* region, const LLParcel* parcel) const\r\n{\r\n\treturn (region && region->getAllowDamage())\r\n\t\t&& (parcel && parcel->getAllowDamage());\r\n}\r\n\r\nSHOULD BE\r\n\r\nbool LLViewerParcelMgr::allowAgentDamage(const LLViewerRegion* region, const LLParcel* parcel) const\r\n{\r\n\treturn (region && region->getAllowDamage())\r\n\t\t|| (parcel && parcel->getAllowDamage());\r\n}\r\n\r\n\r\nIt has completely resolved the issue for me, and avatar health indicators show up fine now.", 'What were you doing when it happened?': '---', 'What were you expecting to happen instead?': '---', } ```
sl-service-account commented 8 years ago

Schneebly Boyd commented at 2016-08-21T03:38:04Z

First. Can confirm this issue on current source release.

sl-service-account commented 8 years ago

Whirly Fizzle commented at 2016-08-21T14:25:12Z

Heya Moy,

Linden Lab actually changed the damage behaviour on purpose because it was requested in issue BUG-992. BUG-992 isn't public but basically it was a complaint from an estate owner who did not like that a parcel owner on his estate was able to enable damage at the parcel level when damage was estate disabled. As you know, this caused the LL damage system to be unusable by LL viewer users - see BUG-9422.

LL have actually come up with a really good proposal that will fix damage and keep both sides happy. This is discussed on this TPV meeting recording: https://www.youtube.com/watch?v=dtJsMIn51Fo (relevant part at 11:16 - 19:16).

The new LL proposal is as follows:

Ref: http://wiki.secondlife.com/w/index.php?title=TPVD/Developers_Group_Agenda&oldid=1197137

sl-service-account commented 8 years ago

Moy Loon commented at 2016-08-21T14:43:57Z, updated at 2016-08-21T14:45:20Z

@Whirly Fizzle

The thing is though, is that there is no such thing that can disable damage via the sim. If either of these options are on, you can get killed in the sim, so hiding the damage makes no sense at all.

I don't think I've seen damage icons display properly in ages, since V2 came out. I thought they were just removed at the little status bar at the top of the screen, and just shown in about land. But seeing the broken logic in the patch I listed would explain why it didn't work, because it would need both parcel, and sim-wide damage to be enabled.

Edit: After watching that video, this doesn't affect anything in my bug post, because they are talking about something that doesn't exist.

sl-service-account commented 8 years ago

Whirly Fizzle commented at 2016-08-21T15:03:21Z

I don't think I've seen damage icons display properly in ages, since V2 came out.

Yep, agree. There were problems with the damage icon display, even before the changes made to "fix" BUG-992. The old bug with the damage icon sometimes showing incorrect status is filed at VWR-11671

sl-service-account commented 8 years ago

Moy Loon commented at 2016-08-21T15:11:00Z

@Whirly Fizzle

I believe I have stumbled upon that link before making this, but because of the dumb policy on not being able to comment or modify anything on the Jira means that I cannot provide any information. So I just have to make another post, in hopes a pair of eyes with the power to do so glances upon it.