greeduomacro / runuomondains

Automatically exported from code.google.com/p/runuomondains
0 stars 0 forks source link

Talisman are not correctly displaying properties, especially "Owned by Noone" #37

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
As I was working on something else, I discovered that talisman are not actually 
displaying the "owned by" tag when not yet owned by anyone.  They should say 
"owned by nobody" per OSI, as a means of letting the user know that it will 
bond with the first person to equip.

The fix starts at line 403 of BaseTalisman.cs
Presently reads:
if ( BlessedFor != null )
    list.Add( 1072304, BlessedFor.Name != null ? BlessedFor.Name : "Nobody" );
Should read:
if ( Blessed )
    list.Add( 1072304, BlessedFor != null ? BlessedFor.Name : "Nobody" );

Also, to be OSI correct (and what makes sense in my mind), the "owned by" tag 
should be at the bottom of the property list (so the if statement should be 
moved to the end of the method).

Finally, I'd like to suggest two other changes:
1.  There is currently redundant blessing occurring in the case of Owned By.  
In BaseTalisman, at line 257 and 258, you can see that both loottype.blessed 
and blessedfor are being set.  This is, as I understand it, unnecessary to 
protect the item.  To compare, I generated and used a Personal Bless deed, and 
found that the props are only set for BlessedFor, where the loottype is left as 
"regular".  I suggest the removal of line 258:
    LootType = LootType.Blessed;
This would remove the redundant tagging of "Blessed for so and so" followed by 
"Blessed" to reduce the number of lines being displayed to the user.

2.  The Mondain's Legacy requirement  property should only be displayed in the 
case of users without ML (informing them that they will need to get ML in order 
to use the item).  I'm not certain or familiar on this one, but I seem to 
recall that it's possible to check user flags to determine that.  I know that 
in OSI, you don't see that tag; it seems prudent to reduce the lines of text to 
only what needs to be displayed.  I'm afraid I don't have code to suggest for 
this one, but if it's needed I can look into it.

Thank you for your hard work, it's truly a great contribution to the RunUO 
community, and is very much appreciated!

Original issue reported on code.google.com by xtraora...@gmail.com on 13 Jan 2011 at 11:48

GoogleCodeExporter commented 9 years ago
The talisman code isn't mine.  It is a part of Malganis Mondain's Legacy.  The 
RunUO Devs are working on a new Talisman code so they work as per OSI.  As soon 
as they release the Talismans the whole talisman code will be revamped to match 
RunUO's code.  So those inconsistancies will be fixed at that time.

There is 2 bless codes for 2 different things. LootType blessing is just that.  
Blessing the item so anyone can use it.  And the BlessedFor is for blessing for 
certain individyal characters.  Which allows the name to be placed on the 
talisman.  And restrict usage to that character.

Yes there are flaws in some of the code that does not follow OSI 100%.  There 
are many aspects of RunUO that don't follow OSI 100%.  There is little point in 
me fixing code that will be totally rewritten in the future (hopefully soon).

But I will check the owned by noone tag.

Original comment by ShaiTanMalkier@gmail.com on 14 Jan 2011 at 4:56

GoogleCodeExporter commented 9 years ago
Issue fixed with Owned by tag.  Will be available in next update.

Original comment by ShaiTanMalkier@gmail.com on 14 Jan 2011 at 5:57

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I recognize that a lot of it doesn't match OSI, and I had no idea that the code 
was about to be rewritten.  I, in fact, assumed they were not going to 
re-approach the code or I never would have posted to begin with.
Since I thought it was sort of set the way it was in terms of development, and 
we were already in the code in that very area, I figured I'd just let you know 
as another thing that could be done at the same time.  Maybe I didn't present 
it well, but I'm not looking to fix every single thing to be just like OSI; I 
just thought I'd bring it up since there was a real issue to fix in the same 
spot, and I felt the multiple tags make the item harder to read.  Not a big 
deal, just thought since we were there.

As for the bless tags, blessedfor and loottype.blessed both have the same 
effect, as long as blessedfor is the current character.  The only difference 
between the two is the effect on one character versus any character.  If you 
look at the props on an item that's been targetted with a personal bless deed, 
you can see what I mean.  Blessedfor can handle the needs of blessing and the 
ownedby tag, loottype.blessed isn't necessary.  Now, that said, I understand 
that the codes going to be changed, it's not hurting anything, and you're not 
worried about aesthetic or OSI changes anyway.  Not a problem, just wanted to 
share what I discovered.

Anyway, no worries, none of it's hurting anything, and as you said the talisman 
code is being rewritten, which I didn't know.  Just thought I'd share what I 
found out  since I had already done the work.. it doesn't affect me either way, 
I made the changes on my server already anyway, just thought I'd share with 
everyone else.

Original comment by xtraora...@gmail.com on 14 Jan 2011 at 6:25