Consume via Targeting

[Vorspire]

If you have it assign and store the new target value on the altar item, then nullify it when the targeting is complete or cancelled, you can prevent anyone else from targeting while the target is not null.

 

[Murzin]

it is complex, but not overly complex.

if you skimp on the checks, you could have undesirable behavior.

if you dont call a separate method that handles all the calls, you would run into exploit territory where player A puts in a tmap, player B puts in wmap, and then player B gets the new map.

sub method that does the actual calls and stores the value of the maps ensures that its valid only for the current player

then within that sub method, store the values of the map for the delete after both maps are chosen. otherwise if you delete on target, and they dont have the 2nd map, you have deleted a map and the player gets nothing in return.

it is complex, to not make it complex is to invite problems/complaints/exploits/bugs into your system.

if you do not mind bugs/exploits/problems with your systems, then by all means, do it much simpler, and keep in mind that your players could suffer because you did not want to put in some extra effort.

 

[Pure Insanity]

The method Khaz posted was simple, clean, and would be bug/issue free if he nulled out the values when targeting fails or finishes.

 

[Murzin]

no, khaz's post is why i posted that long bit.

their proposed methods has no controls or checks against exploits.

when i first started scripting for runuo, my first projects were taking other peoples scripts/ideas, and then getting rid of all the bugs and exploits that the original scripter did not take into account, or possibly care about.

if you do not at least try to make your code bulletproof, then you need to expect people to exploit your system. there is a reason i suggested that level of complexity. if you really wanted to, you can make it much more complex to include lists/dictionaries on the altar to track peoples progress and give them the proper reward once they target both maps.

either take into account possible exploits/bugs, or expect your playerbase to abuse them when they get discovered.

 

[Erevan]

I certainly agree with you Murzin about adding in additional checks, etc.. but as I mentioned in my first post, this is one of the more complex scripts I've ever bothered to try my own hand at. Troubleshooting, editing, and the occasional smaller system is easy enough. But this is pushing my abilities, which is why I opted take a more simplistic approach and at least have something, rather than trying my hand at (and likely borking) something a bit more complicated. If you're interested in giving a first-class education on this, by all means, I'm ready and willing to learn. Otherwise, the bit Khaz has offered will have to suffice.

 

[Soteric]

I don't see any exploits in Khaz's code. There should be sanity checks like if map is not null and if it's in player's backpack but other than that his solution is nice. He doesn't store map on altar and other players can't exploit it. Probably there is an obvious exploit but I don't see it.

 

[Erevan]

I'm assuming Murzin is suggesting that if someone sacrifices the first map.. walks away or accidentally untoggles the targeting cursor, someone else could sacrifice the second map without ever doing the first and still get the reward map (which I why I mentioned the idea of putting a timer on the altar as per OSI, in my OP).

 

[Soteric]

In case someone else would double click the altar he would get his unique sequence which doesn't know anything about previous player and his maps.

 

[Khaz]

it is complex, but not overly complex.

if you skimp on the checks, you could have undesirable behavior.

If implemented as-is, your solution would cause the server to freeze almost immediately as control returns to the while loop after the target is sent to the client. I don't see how you can expect to get the targeted object info back into the while loop without doing the whole operation on its own thread and dealing with all that entails from that.

There's no good reason I can see for the altar or any other class to track the map once it's been targeted and deleted.

As Soteric points out, my solution allows multiple players to use the altar at the same time, with their own maps. I would suggest you check that the maps are in the player's backpack on target, but even if they're not, it wouldn't be anything more than annoying for the other players targeting the same map, since the map is deleted once targeted.

 

[Bittiez]

I haven't tested this, but in theory it should all work(In theory).

using System;
using Server;
using Server.Items;
using System.Collections;
using System.Collections.Generic;
using Server.Gumps;
using Server.Mobiles;
using Server.ContextMenus;
using Server.Prompts;
using Server.Targeting;
using Server.Network;
 
namespace Server.Items
{
    public class CorgulAltar : Item
    {
        public bool tmap_done;
        [Constructable]
        public CorgulAltar()
            : base(0x35EF)
        {
            tmap_done = false;
            this.Name = "Sacrificial Altar";
            this.Hue = 2075;
            this.Movable = false;
        }
 
        public override void OnDoubleClick(Mobile from)
        {
            from.SendMessage("Your offering will be consumed by the altar if the sacrifice is accepted. You will then have 30 seconds to re-use the shrine to collect your new map and pay the blood cost.");
            from.Target = new InternalTarget(this, tmap_done);
        }
 
        private class InternalTarget : Target
        {
            private CorgulAltar m_Item;
            public bool tmap_done;
            public InternalTarget(CorgulAltar item, bool t)
                : base(2, false, TargetFlags.None)
            {
                tmap_done = t;
                m_Item = item;
            }
 
            protected override void OnTarget(Mobile from, object targeted)
            {
                PlayerMobile pm = from as PlayerMobile;
 
                if (m_Item.Deleted)
                    return;
 
                WorldMap wmap;
                TreasureMap tmap;
 
                if (targeted is TreasureMap && tmap_done == false)
                {
                    tmap = (TreasureMap)targeted;
                    from.SendMessage("Your offering has been accepted. The price of blood will be taken when you sacrifice your “world map”.");
                    tmap.Delete();
                }
                else if(!(targeted is TreasureMap) && tmap_done == true)
                {
                    from.SendMessage("You must target a treasure map first.");
                    return;
                }
 
                if (targeted is WorldMap && tmap_done == true)
                {
                    from.SendMessage("Your final offering has been accepted and the blood cost has been collected.");
                    from.Hits = 1;
                    from.AddToBackpack(new CorgulMap());
                    wmap.Delete();
                }
                else
                {
                    from.SendMessage("You must target a world map.");
                    return;
                }
            }
        }
 
        public CorgulAltar(Serial serial)
            : base(serial)
        {
        }
 
        public override void Serialize(GenericWriter writer)
        {
            base.Serialize(writer);
 
            writer.Write((int)0); // version
            writer.Write((bool)tmap_done);
        }
 
        public override void Deserialize(GenericReader reader)
        {
            base.Deserialize(reader);
 
            int version = reader.ReadInt();
            tmap_done = reader.ReadBool();
        }
    }
}

 

[daat99]

I haven't tested this and only modified Bittiez code a bit for performance:

protected override void OnTarget(Mobile from, object targeted)
{
if (m_Item.Deleted)
return;
 
bool mapDeleted = true;
if ( tmap_done )
{
TreasureMap tmap = targeted as TreasureMap;
WorldMap wmap = targeted as WorldMap;
if ( tmap != null && !tmap.Deleted )
{
from.SendMessage("Your offering has been accepted. The price of blood will be taken when you sacrifice your “world map”.");
tmap.Delete();
}
else if ( wmap != null !wmap.Deleted )
{
from.SendMessage("Your final offering has been accepted and the blood cost has been collected.");
from.Hits = 1;
from.AddToBackpack(new CorgulMap());
wmap.Delete();
}
else
{
mapDeleted = false;
}
}
if ( !tmap_done || !mapDeleted)
{
from.SendMessage("You must target either a Treasure Map or a World Map first.");
}
}

A note:
You do realize that if the player targets a treasure map than nothing will happen except deleting the map, right?

 

[Murzin]

and here is the problem with doing it that way:

target map 1, it gets deleted
target map 2, it gets canceled because they dont have the 2nd one

whoops, you have now deleted a map from the player without completing the sequence.


and yes, you would probably not want to use the while loop, just try to send them 2 targets and if both values are satisfied, delete the maps and spawn the new map.

 

[Khaz]

Good call, Murzin.

Here's a version that's a little more complicated but maintains state if the player loses the target and will persist through world saving/loading and still allow players to use it simultaneously.

Disclaimer: written in textpad so I dont know for sure that it compiles

public class CorgulAltar : Item
{
    private Dictionary<Mobile, CorgulContext> m_MapTable;
 
    public Dictionary<Mobile, CorgulContext> MapTable
    {
        get{ return m_MapTable; }
        private set
        {
            m_MapTable = value;
        }
    }
 
    [Constructable]
    public CorgulAltar() : base(0x35EF)
    {
        Name = "Sacrificial Altar";
        Hue = 2075;
        Movable = false;
 
        MapTable = new Dictionary<Mobile, CorgulContext>();
    }
 
    public CorgulAltar(Serial s) : base(s) {}
 
    public override void OnDoubleClick(Mobile from)
    {
        if(!MapTable.ContainsKey(from) || !MapTable[from].SacrificedTMap)
        {
            MapTable[from] = new CorgulContext(from);
         
            from.SendMessage("Select a treasure map");
            from.BeginTarget(2, false, TargetFlags.None, new TargetCallback( OnTargetTMap ));
        }
        else if(!MapTable[from].RewardReceived())
        {
            from.SendMessage("You've already sacrificed a treasure map. Now you must sacrifice a world map");
            from.BeginTarget(2, false, TargetFlags.None, new TargetCallback( OnTargetWMap ));
        }
        else
        {
            from.SendMessage("You have already received your reward from this altar.");
        }
    }
 
    private void OnTargetTMap(Mobile from, object targeted)
    {
        if(targeted is TreasureMap)
        {
            MapTable[from].SacrificedTMap = true;
         
            ((TreasureMap)targeted).Delete();
         
            from.SendMessage("Your offering has been accepted. The price of blood will be taken when you sacrifice your \"world map.\"");
            from.BeginTarget(2, false, TargetFlags.None, new TargetCallback( OnTargetWMap ));
        }
        else
        {
            from.SendMessage("No, pick a treasure map");
            from.BeginTarget(2, false, TargetFlags.None, new TargetCallback( OnTargetTMap ));
        }
    }
 
    private void OnTargetWMap(Mobile from, object targeted)
    {
        if(targeted is WorldMap)
        {
            MapTable[from].SacrificedWMap = true;
         
            ((WorldMap)targeted).Delete();
         
            from.SendMessage("Your final offering has been accepted and the blood cost has been collected.");
            from.Hits = 1;
         
            from.AddToBackpack(new CorgulMap());
        }
        else
        {
            from.SendMessage("No, pick a world map");
            from.BeginTarget(2, false, TargetFlags.None, new TargetCallback( OnTargetWMap ));
        }
    }
 
    public override void Serialize(GenericWriter writer)
    {
        base.Serialize(writer);
     
        writer.Write((int)0);
     
        writer.Write((int)MapTable.Count);
     
        foreach(KeyValuePair<Mobile, CorgulContext> kvp in MapTable)
        {
            writer.WriteMobile<Mobile>(kvp.Key);
            kvp.Value.Serialize(writer);
        }
    }
 
    public override void Deserialize(GenericReader reader)
    {
        base.Deserialize(reader);
     
        int version = reader.ReadInt();
     
        switch(version)
        {
            case 0:
                int tableCount = reader.ReadInt();
             
                MapTable = new Dictionary<Mobile, CorgulContext>(tableCount);
             
                for(int i = 0; i < tableCount; i++)
                {
                    MapTable[reader.ReadMobile<Mobile>()] = new CorgulContext(reader);
                }
             
                break;
        }
    }
 
    private class CorgulContext
    {
        private Mobile m_Mobile;
        private bool m_TMap;
        private bool m_WMap;
     
        public Mobile Mobile
        {
            get{ return m_Mobile; }
            set{ m_Mobile = value; }
        }
     
        public bool SacrificedTMap
        {
            get{ return m_TMap; }
            set{ m_TMap = value; }
        }
     
        public bool SacrificedWMap
        {
            get{ return m_WMap; }
            set{ m_WMap = value; }
        }
     
        public CorgulContext(Mobile from)
        {
            m_Mobile = from;
        }
     
        public bool RewardReceived()
        {
            return (SacrificedTMap && SacrificedWMap);
        }
     
        public void Serialize(GenericWriter writer)
        {
            writer.Write((int)0);
         
            writer.WriteMobile<Mobile>(Mobile);
            writer.Write(SacrificedTMap);
            writer.Write(SacrificedWMap);
        }
     
        public CorgulContext(GenericReader reader)
        {
            int version = reader.ReadInt();
         
            switch(version)
            {
                case 0:
                    Mobile = reader.ReadMobile<Mobile>();
                    SacrificedTMap = reader.ReadBool();
                    SacrificedWMap = reader.ReadBool();
                 
                    break;
            }
        }
    }
}

 

[Soteric]

Khaz, you can use TargetStateCallback and pass previously targeted map to the next target. Then validate that map wasn't deleted between targets and still in player's backpack and delete it.

 

[Khaz]

Khaz, you can use TargetStateCallback and pass previously targeted map to the next target. Then validate that map wasn't deleted between targets and still in player's backpack and delete it.

Yes, but if the WMap target is cancelled or lost, the player would need to start over from the beginning. Not a big deal, though, since that solution would be much simpler.

 

[Vorspire]

Khaz, you should get ReSharper, your code is great but still has issues with accessibility and redundancy;

1 Using directive is not required by the code and can be safely removed
3 Using directive is not required by the code and can be safely removed
4 Using directive is not required by the code and can be safely removed
5 Using directive is not required by the code and can be safely removed

22 Inconsistent accessibility: type argument 'Server.CorgulAltar.CorgulContext' is less accessible than property 'Server.CorgulAltar.MapTable'

51 Redundant explicit delegate creation
56 Redundant explicit delegate creation
73 Redundant explicit delegate creation
78 Redundant explicit delegate creation
98 Redundant explicit delegate creation
106 Type cast is redundant
108 Type cast is redundant
112 Type argument specification is redundant
175 Type cast is redundant
177 Type argument specification is redundant

While everything else is probably .NET 3+ specific and can't be helped, the accessibility issue on line 22 would make the script fail to compile.

This is because the CorgulContext class is accessible via the Dictionary from outside of the CorgulAltar class, meaning it can't be private, the next best way to limit it is;

public sealed class CorgulContext

Or never make the Dictionary publicly accessible.

 

[Khaz]

Disclaimer: written in textpad so I dont know for sure that it compiles

Yup

If I was writing it for myself it would've been in VS and fully tested... Got to leave some work for the OP

 

[Vorspire]

I just wish RunUO used .NET 4 as standard, I'm so used to writing .NET 4 compliant code that giving .NET 2 help out on these forums is becoming quite a challenge, lol.

This altar item could be done in half the lines of code, starting with auto-properties, and maybe tuples instead of nested custom classes.

using CorgulContext = System.Tuple<Server.Mobile, System.Boolean, System.Boolean>;

 

[Erevan]

lol.. you guys are having too much fun debating all of this! Let me clarify, I'm using full .NET 4.0 support (from the ForkUO fork). So no need to fear having to offer assistance based on old 2.0 code.

 

[Khaz]

I just wish RunUO used .NET 4 as standard, I'm so used to writing .NET 4 compliant code that giving .NET 2 help out on these forums is becoming quite a challenge, lol.

This altar item could be done in half the lines of code, starting with auto-properties, and maybe tuples instead of nested custom classes.

using CorgulContext = System.Tuple<Server.Mobile, System.Boolean, System.Boolean>;

Yup! I actually wrote it first with auto-generated properties and then some, but realized it wouldn't be 2.x compliant and changed it before posting

lol.. you guys are having too much fun debating all of this! Let me clarify, I'm using full .NET 4.0 support (from the ForkUO fork). So no need to fear having to offer assistance based on old 2.0 code.

Good deal... there are certainly pieces in the example you can simplify!