Redundant code or a mistake?

[Elg'cahlxukuth]

Hi!

I'm reading the Fishing.cs file to get some knowledge about how fishing skill works. I can see that inside the Construct method of the Fishing class there is a code which generates a treasure chest of some level:
TreasureMapChest.Fill( chest, Math.Max( 1, Math.Max( 4, sos.Level ) ) );

I wonder if there should rather be:
TreasureMapChest.Fill( chest, Math.Max( 1, Math.Min( 4, sos.Level ) ) );

The original code make no sense to me. I mean it is the same as just:
TreasureMapChest.Fill( chest, Math.Max( 4, sos.Level ) );

I suspect that the autor wanted to constrain the value of the Fill method parameter to be from 1 to 4.
It is hard for me to tell what values are proper here, as I have no idea of how exactly fishing skill should work.

It is in RunUO 2.2 and 2.0 (and maybe others).

Sorry for my english :-/ I hope it can be understood.
Regards, Darek

 

[Vorspire]

Yea, that looks like a bug, that second Max call will always result in level 4 being the lowest possible integer, IIRC, SoS level max is actually 4.

The intention was definitely to constrain the values between 1 and 4.

The correct code should be;

Math.Max( 1, Math.Min( 4, sos.Level ) );

 

[Xavier]

this was fixed in revision 867, April 2012.

 

[Vorspire]

Is it worth releasing a RunUO 2.3 yet Xavier?
Lots of people are taking the 2.2 release as the most recent version when the SVN is so much further ahead.

 

[Xavier]

I was discussing this with Ryan not long ago, and I think we both agreed - we need to come up with a new official package.

In the meantime, Id be happy to package up an updated rc when I push updates to the public SVN, as long as everyone is good with that. I agree, its definitely needed. Ive been advising people to just c/o the svn.