jpcsupplies / Economy_mod

Basic Economy System for Space Engineers
13 stars 12 forks source link

Toby Branch /value command #42

Closed jpcsupplies closed 8 years ago

jpcsupplies commented 8 years ago

Toby was in today and had a crack at the /value command after i commented in some pseudo logic into the master earlier.. he got his code pulling up the values although there are a few aethetic issues with replies.. it seems to work, just needs some regex/pattern matching and lowercase converting..

He was hoping you could look over his code and give him some feedback on iit. https://github.com/jpcsupplies/Economy_mod/tree/TobyValueFeature

To me it looks like his not fully utilising the code you put in for price table as efficiently as he could, but he went to the trouble of putting in server / client side voodoo, so its a good effort, certainly better than me.

jpcsupplies commented 8 years ago

also we are getting to the point where we probably should create a ref/const/function table in the wiki for detailing what all the different elements we are using to pull up stuff is.. like the main stuff like bankconfigdata.accounts etc

it was easy when i took a crack at the bal and pay commands as it was mostly all there in the file in front of me, but now we have 26 files; I am struggling to keep track of what calls and does what where.. ive lost the plot! lol i was thinking the value command would be like bal, just pull up the record and confirm its there, then throw the reply to the screen (and it probably is that easy) but i got totally lost trying to work out what i needed to refer to; in order to compare to the data; i was hoping to try to use your tidy style that you converted the bal/pay etc commands into (ie regex, lookup check reply) since thats pretty clean to look at but its a bit of a learning curve at the moment.

EDIT: ill probably take a crack at this.. just need to list all the functions/arrays and what not we use to hold all our key data... eg bank balances, item prices etc

midspace commented 8 years ago

PopulateItems should not be there. We already have real data in there. The Market item list is already accessible via EconomyScript.Instance.MarketConfigData.MarketItems None of relates to components of which DisplayNames can be retrieved from Localization. None of the code is utilizing TypeId which is required in conjunction with SubtypeName. There is no need to wrapper Add, Remove, List. You can access the List<> directly with those commands. No need to add "Begin Tobys Code" and "End Tobys Code". Git source control tells you who added what. All you are doing is making it harder to read the code.

SaltPepp commented 8 years ago

It was a good try coming from someone who plays Space Engineers very little and I added the code to existing code i didn't create.

Keeping in mind, i created my code externally from the mod originally and i just copied and pasted it into the mod and made minor adjustments.

The code before it got placed in the mod was designed to work as a c# console application and not economy mod connected at all. That explains why we have two game item lists as i made my code work externally outside the game if that makes sense.

My wrapper for lists is to give me alternative ways of working with data. That's why you may see the same function name more then once in my code but different parameters.

Sent from my Windows Phone


From: midspacemailto:notifications@github.com Sent: ‎16/‎09/‎2015 7:10 PM To: jpcsupplies/Economy_modmailto:Economy_mod@noreply.github.com Subject: Re: [Economy_mod] Toby Branch /value command (#42)

PopulateItems should not be there. We already have real data in there. The Market item list is already accessible via EconomyScript.Instance.MarketConfigData.MarketItems None of relates to components of which DisplayNames can be retrieved from Localization. None of the code is utilizing TypeId which is required in conjunction with SubtypeName. There is no need to wrapper Add, Remove, List. You can access the List<> directly with those commands. No need to add "Begin Tobys Code" and "End Tobys Code". Git source control tells you who added what. All you are doing is making it harder to read the code.


Reply to this email directly or view it on GitHub: https://github.com/jpcsupplies/Economy_mod/issues/42#issuecomment-140679915

jpcsupplies commented 8 years ago

:+1:

SaltPepp commented 8 years ago

To add onto what I said, Atleast I got the job done of adding a working /value command As I said before, I'm not a effiecent programmer and I tend todo stuff in a dirty way but I get the job done in the end. Thankyou for your feedback @midspace, I can use this feedback in the future.

jpcsupplies commented 8 years ago

I think - correct me if I am wrong, midpace used a fair bit of your logic in the master branch implementation ? so it was a great effort.. its in the mod now.

midspace commented 8 years ago

If I come across a little blunt, that's just me, and I don't intend to. I do prefer to say thing as how they are (no coloring or looking through rosey glasses), because I believe communication between people to be important, because people can easily leave things out or just not mention them thinking they aren't important to a conversation or might belittle the other person.

Doing a code review for other programmers is a normal part of the process these days. Been able to point out code logic issues, inefficiencies, or where things could be improved for future maintenance always helps. If you are doing Agile Pair programming, there will be two of you at a single computer contributing to the codebase at the same time. :scream: https://en.wikipedia.org/wiki/Pair_programming

I do know that my code here only partially covers localization, but that is because we only have component names localized currently. If we had fully access to localizing, I would be restructuring this code significantly. But if I did that, it would be much harder for both of you to follow the logic. As it is, it's not necessary right now for the mod to work, so it's considered Technical Debt. https://en.wikipedia.org/wiki/Technical_debt

I'm taking my lead from @jpcsupplies but yes I used much of @SaltPepp code logic. I kind of feel like a freelancer, which is fine with me. :stuck_out_tongue_winking_eye:

jpcsupplies commented 8 years ago

Ok, going to get wishy washy here .. and blunt is good. I think midspace is doing a great job, I get the impression, just the way they work, i see it in their code, that they have had to deal with many programmers(?) before, the way they incorporate code and features is done in a very diplomatic way. Like even my crude monkey mashing of keys type code; usually features in part in the final result, even if it is surrounded by midspaces touches of absolute brilliant coding, I see little bits of mine still there too. I know I am totally outclassed in the programming side here, yet I feel flattered that my contribution still counted.

Myself, having worked previously in an environment where the programmers egos had to be treated carefully; and they took bug reports much too personally sometimes, getting that balance of diplomacy vs your own frustration at being yelled at by clients at the helpdesk all day is tricky.

Midspace just seems to nail that balance each time; If i was to write a reference letter for them, I would probably say, they are a highly efficient very skilled programmer, who works brilliantly in a team, but is also great at innovating solutions on their own when necessary.

I sit here sometimes after looking at midspaces work, and wonder how I managed to get such a world class programmer on my little project..

Then i think, damn.. there is a pizza shop somewhere in Melbourne that is going to need a bigger delivery truck one day ROFL.. :O

jpcsupplies commented 8 years ago

And to Toby, don't undersell yourself, You have already built several websites, including a complete fast food ordering system.. with payment and merchant facilities - and its making them money. Its an excellent start in software development. I get you didn't want to charge too much for it, as you were learning as you went along; but what you built was probably worth over $15,000 to that business... had any formal website company done it - and at its current turnover their ROI on $15,000 would have been about 30 weeks. That is a spectacular result, even if it is in part hypothetical in this case.

And you can apply this to other businesses now too, so you have built yourself a marketable product, which you CAN probably sell one day to another business for up to $15,000 or more depending who used it. I knew what your potential was when I basically handed you the keys literally and figuratively to my old webhosting business.

Dont put yourself down, like you said you didn't have any experience in space engineers; but you got the job done, and quite quickly i might add. Ok it had some oversights, but for a first effort that was great. At least it ran, my first attempt mod attempt resulted in an F11 log failure, and my second crashed the game entirely, yours did neither@!

Like I said to you here, I only just added this "value" command bit, go nuts, you cant do anything wrong, if you broke anything we can easily roll it back, and if you get it working, that buzz you get when you see it all working in game cant be beaten.

Hell make your girlfriend read this, just to make sure she doesnt undersell you too lol. To use a random pop culture reference - Every girl wants to be the one that got the sk8ter boi from the Avril Lavine song. If you really like her, make sure she doesnt end up the girl in the audience :P

midspace commented 8 years ago

Hey, I regularly crash the game. In fact I was telling @jpcsupplies about it last night, and crashed the game for both of us, and the server too. Don't weld Laser Antenna in survival mode at the moment.

jpcsupplies commented 8 years ago

lol that reminds me i should probably delete that antenna..

midspace commented 8 years ago

OP has been addressed. Closing.