-
Member
First of all this should be considered constructive criticism, no flame
Snippet A:
Code:
var items = Hud.Game.Items.Where(item => CONDITION_X);
foreach (var item in items)
//DoSomething
var items = Hud.Game.Items.Where(item => CONDITION_Y);
foreach (var item in items)
//DoAnotherThing
{ more code conditions }
Snippet B:
Code:
foreach (var item in items)
{
if (CONDITION_X)
DoSomething
if (CONDITON_Y)
DoAnotherThing
{ more code conditions }
}
Wouldn't the second code-snippet be the better approach especially in regard to the multiple where-selections and the tick-engine functionality of THUD.
The benefit from Snippet B should improve with more and more items.
Snippet B is more confusing in regard to code structure but for the sake of clarity you can encapsule CONDITION_X & CONDITION_Y as well as DoSomething and DoAnotherThing into methods.
Lets assume the following situation:
Items = 100
10 from 100 with Condition_X
30 from 100 with Condition_Y
Snippet A
Items.Where(item => CONDITION_X) => 100 iterations
foreach (var item in items) => 10 iterations
Items.Where(item => CONDITION_Y); => 100 iterations
foreach (var item in items) => 30 iterations
Snippet B
foreach (var item in items) => 100 iterations
Last but not least top plugin keep up the good work
Last edited by Xqzmeplz; 09-09-2017 at 10:38 PM.
-
Post Thanks / Like - 1 Thanks
User5981 (1 members gave Thanks to Xqzmeplz for this useful post)
-
Active Member
The customization of the plugin doesn't work. I think the code is old? Didn't find a "true/false if" in the code.
-
First Dev On The Internet
Originally Posted by
bm206
The customization of the plugin doesn't work. I think the code is old? Didn't find a "true/false if" in the code.
Code has been updated when I added customization, redownload copy/ paste it. in the 1st post.
-
Active Member
It is working if i change true to false in your plugin code but not with customization (no exceptions -> my code should be ok)
-
First Dev On The Internet
Originally Posted by
Xqzmeplz
First of all this should be considered constructive criticism, no flame [cut to avoid long quote]
Ok, to be honest I started C# one week or so after discovering TurboHUD a few weeks ago, modified ItemsPlugin.cs before understanding why making plugins and how, I consider myself more an advanced user than a dev (I do a lot of copy paste from other plugins + minor modifications), I'm having more fun and brain stimulation in making plugins than actually playing the game.
I'm really interested in hints like this to help me to improve my coding because I miss basic knowledge.
So what I understand in your post is that each foreach loop is a request to a database and to avoid memory overload it is better to query a larger part of the database once and apply all the conditions after that.
I will try to improve that, thank you
-
Active Member
Does C# have a 'case' statement like C? If yes, it would be better to use it because as soon as it finds one condition to be true, all others will be ignored. The way Xqzmeplz posted, all 'if' statements will be checked even if a previous one was true.
Edit: C# does have it: switch keyword (C# Reference) | Microsoft Docs
Last edited by johnbl; 09-10-2017 at 08:57 AM.
Reason: Googled the info
-
First Dev On The Internet
less screen pollution update! (explanation in the 1st post changelog)
-
Post Thanks / Like - 1 Thanks
cherouvim13 (1 members gave Thanks to User5981 for this useful post)
-
Contributor
Originally Posted by
bm206
It is working if i change true to false in your plugin code but not with customization (no exceptions -> my code should be ok)
@User5981
your customizations cannot work.
these statements are to "late".
because Hud loaded your plugins already, where you defined your "SnoMapping" Dictionary.
e.g.
PHP Code:
plugin.Gems = false;
this sets "Gems" after loading your plugin to false, but in "loading-time" it was plugin default "true", so the dictionary contains all those gem_snos.
afterwards setting "Gems" to "false" will not remove the sno from the dictionary. and "Gems" is not used after loading time, so its useless to alter with a customization.
either we alter the plugins bools in original file,
or we have to do stuff like this:
PHP Code:
plugin.SnoMapping.Remove(2979276674); // remove a gem from list
greetz gjuz
-
First Dev On The Internet
Originally Posted by
gjuz
@User5981
your customizations cannot work.
these statements are to "late".
because Hud loaded your plugins already, where you defined your "SnoMapping" Dictionary.
e.g.
PHP Code:
plugin.Gems = false;
this sets "Gems" after loading your plugin to false, but in "loading-time" it was plugin default "true", so the dictionary contains all those gem_snos.
afterwards setting "Gems" to "false" will not remove the sno from the dictionary. and "Gems" is not used after loading time, so its useless to alter with a customization.
either we alter the plugins bools in original file,
or we have to do stuff like this:
PHP Code:
plugin.SnoMapping.Remove(2979276674); // remove a gem from list
greetz gjuz
ok I get it now,
will something like this work in the plugin ? (in order to keep the customization easy for users):
PHP Code:
if (!ForgottenSoul) { SnoMapping.Remove(2073430088);}
Edit : I have it working, will update soon, thanks
Last edited by User5981; 09-12-2017 at 01:10 AM.
-
Contributor
i could suggest an init() funktion where you fill your dictionary once, after loading.
something like this:
PHP Code:
private bool init_mapping; //set to false in load(...)
private void init() { //fill your dictionary
//Death Breath => already handled by ItemsPlugin but needed
if (DeathsBreath) { AddDecorator(2087837753, Hud.Render.CreateBrush(160, 89, 178, 153, 0), whiteBrush, orangeFont); }
//and so on
init_mapping = true;
}
public void PaintWorld(WorldLayer layer)
{
//new for init
if(!init_mapping) { init(); }
//normal as before
var itemGroups = Hud.Game.Items.Where(item => item.Location == ItemLocation.Floor).GroupBy(item => item.SnoItem.Sno);
//...
greetz gjuz
-
Post Thanks / Like - 1 Thanks
User5981 (1 members gave Thanks to gjuz for this useful post)
-
First Dev On The Internet
Optimization now fully working! Thanks a lot to gjuz!
-
Member
Came across a bug I guess...was cleaning out my inventory and dropped 2 ancients and saw this on my minimap. I cleared my plugins and added one by one until it popped up with yours...any ideas? I don't mind the labels or maybe I do, not sure but the dupes def bother me lol.
Tried putting false on diff lines but nada unless its line 140 but then no label on said drops and no dupes...
Last edited by Runn1ng_F4st; 09-12-2017 at 03:53 AM.
-
Contributor
Originally Posted by
User5981
Optimization now fully working! Thanks a lot to gjuz!
u missunderstood me a little bit,
it works for sure, but i meant that u place all code which >>fills<< the dictionary to init() and leave everything else like the brushes in load(...).
so move (my opinion)
PHP Code:
SnoMapping = new Dictionary<uint, WorldDecoratorCollection>();
var blackBrush = Hud.Render.CreateBrush(160, 0, 0, 0, 1);
....
var grayFont = Hud.Render.CreateFont("tahoma", 7, 255, 55, 61, 53, true, false, false);
back to load
greetz gjuz
-
First Dev On The Internet
Originally Posted by
Runn1ng_F4st
Came across a bug I guess...was cleaning out my inventory and dropped 2 ancients and saw this on my minimap. I cleared my plugins and added one by one until it popped up with yours...any ideas? I don't mind the labels or maybe I do, not sure but the dupes def bother me lol.
Tried putting false on diff lines but nada unless its line 140 but then no label on said drops and no dupes...
That's not really a bug, the default item plugins will not display legendary items icons on minimap when in town (if I remember correctly)
but my plugin does work in town so you have the ancient indicator but not the icon.
I'm not sure if I should disable the whole plugin in town (if another player in game drops something interesting in town you'll have to use ctrl or alt to actually see it) or keep it or make it an option ...
Last edited by User5981; 09-12-2017 at 04:17 AM.
-
First Dev On The Internet
Originally Posted by
gjuz
u missunderstood me a little bit,
it works for sure, but i meant that u place all code which >>fills<< the dictionary to init() and leave everything else like the brushes in load(...).
so move (my opinion)
PHP Code:
SnoMapping = new Dictionary<uint, WorldDecoratorCollection>();
var blackBrush = Hud.Render.CreateBrush(160, 0, 0, 0, 1);
....
var grayFont = Hud.Render.CreateFont("tahoma", 7, 255, 55, 61, 53, true, false, false);
back to load
greetz gjuz
I first did that but I had exceptions where values like blackBrush, blackfont ...where not existing in that context.