|
stuck
Mar 18, 2009 21:32:55 GMT
Post by bulldogg on Mar 18, 2009 21:32:55 GMT
ok i have this code, its cut down to show only whats needed pastebin.com/m537bce9cBasically this code allows players to send a message to universe for a fee. it gets the fee price from an xml and sets the variable creditloss to the price it will cost. Basically im going ingame to test this and my player has 240 credits, and my charge fee is 1000 else if (creditloss >= currentcash) { PrivateChat.Fire(user, string.Format("You do not have the required credits to perform this action. You require another {0} to send a banner.") ); return; } this code above checks that if the fee is larger than the players current cash it will show an error and cancell the action. But it doesnt seem to work, it just carries on and sends the message and doesnt deduct anything. Would using the AddCash callback help at all? not sure what that does actually lol. im not too sure why creditloss >= currentcash isnt working BD
|
|
|
stuck
Mar 19, 2009 9:07:15 GMT
Post by Eagle on Mar 19, 2009 9:07:15 GMT
This requires a better understanding of what happens, what the callbacks are for etc. I've copied your code snippet and added comments below. I also added some tips:
internal class PlayerU: PlayerCommand { public PlayerU() { Syntax += " [message]"; Description = "Allows players to pay to send out a universal message."; }
// Never use fields in this fasion, these should be private or at // the very least protected. Public fields defy the concept of encapsulation, // you should hide the inner workings of your classes. public string logpath = Environment.GetFolderPath(Environment.SpecialFolder.Personal) + @"\FLAntiCheat\universepaylog.log"; public int creditloss = 0; public int currentcash;
// The purpose of this class is to translate a player command, any // other functionality should have its own class. This is known as seperation // of concerns, the method below should be part of a Settings class instead // and furthermore private as the actual fetching from persistant storage is // of no concern to any other classes, nor should other classes be able // to determine when settings are read as this is again a concern of the // Settings class itself. public void GetSettings() { XmlDocument document = new XmlDocument(); document.Load(Environment.GetFolderPath(Environment.SpecialFolder.Personal) + @"\FLAntiCheat\payperbannerconfig.xml"); XmlNode node = document.DocumentElement.SelectSingleNode("//Price"); creditloss = Convert.ToInt32(node.InnerText.ToString()); }
public override void Execute(Player user, string parameters) { GetSettings();
// This is where things go south, what happens in this bit of // code is that an event is fired into the server asynchronously. This // means that the actual event is processed on the server whilst your code // is allowed to continue. The callback delegate is basically just a new // method which is called upon by the server as soon as it completes the // associated event. This method might take say a millisecond or so before // its called upon after the related event was fired. This means that the // rest of your Execute method has allready been processed before the // delegate is executed to set the proper amount. GetCash.Fire( user, delegate(CashUpdate notification) { currentcash = notification.Amount; } );
// currentcash is always zero at this point as the fired event // has not been executed on the server yet. You should also avoid running // bits of code you do not need, if the player has enough cash, or // neglected to enter a message, he will not be prompted for the amount // calculated here. It should therefor be embedded in the section dealing // with not having enough credits instead. int requiredcash = creditloss - currentcash;
// Another seperation of concerns violation, it is of no // interrest to this class wether or not some log file exists. This is the // sole responsibility of a Logger class, all you should be able to do is // call upon a method of a Logger class to log text, how or when this // Logger then decides to append or overwrite files is again the // responsibility of a Logger class. Adhering to such specific seperation // allows you to easier add complex functionality for each class manages // its own state of things. Currently your class is riddled with code that // does not belong, making it hard to maintain especially when having to add // afore mentioned compex functionality. if(!File.Exists(logpath)) { CreateLog(); }
// At this point you're checking for a possible input error, // which might lead to termination of this method entirely. If we are // indeed to terminate here, we've done all that hard work at this // beginning of this method for nothing. This check should have been done // up front as to not execute unneccessary code, this concept is known as // fail as early as possible. if(parameters == "") { PrivateChat.Fire(user, "You did not enter a message to send."); return; } // The else part is redundant as the if branch immediatelly // exits this method (return), therefor else should have been left out entirely. The // comparisson made in itself is perfectly fine, I however prefer reading // from left to right so I'd have put currentcash <= creditloss instead. // Personal taste, thought I find it makes reading of more complex if // statements alot easier. else if(creditloss >= currentcash) { PrivateChat.Fire( user, string.Format("You do not have the required credits to perform this action. You require another {0} to send a banner.") ); return; }
// This is what the command is supposed to do when all criteria // have been met, placing this in a seperate, private!, method would make // reading of the execute method easier, plus it would keep your methods // short. Always try to keep your methods short, if they exceed 10 lines of // code, you're probably stuffing too much in a single method. All of the // code below could be placed in a method called SendToUniverse. // // Much more important is the fact that you did not use a callback for the // AddCash event. This callback tells you wether or not the credits were // deducted, it is afterall possible that by the time the event is // processed on the server, your target player is offline. This is just one // of many reasons why credit deduction could fail, so its always best to // verify before performing actions that require payment. // // BUG: // Up to here creditloss has been a positive figure (0 < creditloss). // The AddCash event would therefor add the amount due, rather than subtract. // You would want to alter the parameters to (user, -creditloss) instead. EagleWare.Libraries.FLServerCommunications.Events.AddCash.Fire(user, creditloss); UniverseCustomChat.Fire(0xFFFFFF30, user.Name + ": " + parameters); // A string variable is created, only to be used in the next statement // and then never again. Try to avoid creating unnecessary variables, e.g.: // // WriteToLog(string.Format("Player: {0}, Credit Charge: {1}, Message: {2}.", user.Name, creditloss, parameters)); string entry = string.Format("Player: {0}, Credit Charge: {1}, Message: {2}.", user.Name, creditloss, parameters); WriteToLog(entry); PrivateChat.Fire(user, string.Format("Message sent to universe. You have had {0} credits deducted from your account to submit the message '{1}'.", creditloss, parameters)); } }
|
|
|
stuck
Mar 24, 2009 16:35:47 GMT
Post by bulldogg on Mar 24, 2009 16:35:47 GMT
right ok ill work on this when ive got some free time
one thing i still done fully understand is how to use
GetCash.Fire( user, delegate(CashUpdate notification) { currentcash = notification.Amount; } );
this to work for me
BD
|
|
|
stuck
Mar 24, 2009 19:53:29 GMT
Post by Eagle on Mar 24, 2009 19:53:29 GMT
The method of calling GetCash.Fire below is how I usually write code, instead of a full blown method I use what's called an anonimous delegate. Anonimous delegates are a very powerfull tool in .NET but they're a bit hard to understand if you're unfamilliar with callback methods.
public override void Execute(Player user, string parameters) { Print("About to execute GetCash.Fire"); GetCash.Fire( user, delegate(CashUpdate notification) { Print("In callback."); // Do something with notification.Amount; } ); Print("GetCash.Fire has been executed."); }
The above is equivalent to:
private void ProcessCashUpdate(CashUpdate notification) { Print("In callback."); // Do something with notification.Amount; }
public override void Execute(Player user, string parameters) { Print("About to execute GetCash.Fire"); GetCash.Fire(user, ProcessCashUpdate); Print("GetCash.Fire has been executed."); }
The ProcessCashUpdate method is 'called back' by FLAC when the server has processed the request to retrieve the amount of credits 'user' has. Calling GetCash.Fire will not immediatelly result in ProcessCashUpdate being executed, infact... statements following will be executed almost immediatelly after GetCash.Fire was called. The GetCash.Fire method merely sends out a request to retrieve a cash amount, it does not actually inspect cash nor wait for this to be performed on the server. This is what's called an asynchronous call. Any logic that depends on the amount a player has should therefor be embedded in the ProcessCashUpdate method instead.
If Print was to actually output text then calling Execute would result in the following printed output:
About to execute GetCash.Fire GetCash.Fire has been executed. In callback.
|
|