Wulf2k / DaS-PC-MPChan

77 stars 30 forks source link

Check online state of stored nodes #4

Closed Chronial closed 8 years ago

Chronial commented 8 years ago

This checks the steam api to see whether the stored nodes are currently playing Dark Souls. It uses a proxy script of mine, as I don't think you are allowed to distribute the API Keys for the Steam Web API.

Wulf2k commented 8 years ago

At a quick glance, it looks like the Favorites loop will overwrite the SteamIDs of the Recent entries in intIds().

Do I just need caffeine or is that an oversight?

Wulf2k commented 8 years ago

Also, how solid is your hosting for that script?

My server gets a request for the dscm-ver.txt file roughly every 5-10 seconds, and that only happens when the tool is first opened. Do you think your server can handle that level of sustained activity? How much processing happens on your end?

Wulf2k commented 8 years ago

I'm not sure if you'll get notifications on my reply to your line note, but if recent nodes were capped at 1000, I imagine this would put a rather large strain on your server.

Wulf2k commented 8 years ago

I'm definitely not opposed to this update, but I'd like to hear back from you before accepting it.

Chronial commented 8 years ago

You were right on the loop – fixed.

Regarding server load: Yes, that server can definitely handle that, and there is barely any processing done anyways. The only limit this might hit is the steam api limit, which is 100000 requests per day. With the current settings this will be fine if the average runtime of the tool is about one hour.

If we hit the limit, I will have to do some caching on the server.

Wulf2k commented 8 years ago

Looks good so far, but I'd like to see it in action before accepting it.

Should be able to try it out tonight and I'll commit then if all goes well.

Thanks.

Wulf2k commented 8 years ago

Won't adding to the HashSet fail when a member of Favorites is also in your Recent list?

Edit: After a bit of reading it looks like the add attempt should return a boolean false if the item already exists, but (hopefully) not fail in any serious manner.

Chronial commented 8 years ago

Nope, see documentation: https://msdn.microsoft.com/en-us/library/bb353005%28v=vs.110%29.aspx

Doing things like that is exactly the point of a hashset ;).

Wulf2k commented 8 years ago

It seems to work pretty well so far, but last night I came across a player in the Recent list that was showing offline even though I was connected to him for over 10 minutes.

Any thoughts on why that could be?

https://steamcommunity.com/id/CEnigma

Chronial commented 8 years ago

Yes, this is completely expected – they activated the relevant privacy settings, so we can't get their game online status from steam. I think the steam API allows us to detect that https://developer.valvesoftware.com/wiki/Steam_Web_API#GetPlayerSummaries_.28v0002.29 (communityvisibilitystate). For now it seemed good enough to just report whether this player is surely online.

Here's the proxy script btw: https://gist.github.com/Chronial/a8b2ec3546a0efeb4bf3bc1f875bf8ae, so this can also easily be hosted elsewhere should this be required. I would just push my git repo to github, but I'm too lazy to separate my key from the git history :).

I also wanted to point out that I'm not married to that code – feel free to rip it apart and change it up as you feel fit. I just wanted to add this feature.

General note: I think dscm is missing a clear page with release information and other info. I stumbled upon your awesome release post in forum, that even included a video (http://steamcommunity.com/app/211420/discussions/0/364039785166582725/), but that is really hard to find and I guess most people will miss things like that.

I would propose the use the github releases feature to announce releases (see for example a project of mine: https://github.com/Chronial/foo_chronflow/releases). This is very easy to maintain and would give people a very clear place to link to, where they can get all the information.

I realize you announce releases on http://wulf2k.ca/, but any kind of detailed information (like the awesome tutorial videos you create) is missing from that page.

Wulf2k commented 8 years ago

Good point. I should definitely have a better release procedure.

I'll think on it.

On another note, any idea why some people (but not all) are still getting Int32 errors every 5 seconds?

http://steamcommunity.com/app/211420/discussions/0/364040166667206474/#c364040166667806294

\ Exception Text ** System.ArgumentException: Object must be of type Int32. at System.Int32.CompareTo(Object value) at System.Collections.Comparer.Compare(Object a, Object b) at System.Collections.Generic.ObjectComparer1.Compare(T x, T y) at System.Linq.EnumerableSorter2.CompareKeys(Int32 index1, Int32 index2) at System.Linq.EnumerableSorter1.QuickSort(Int32[] map, Int32 left, Int32 right) at System.Linq.EnumerableSorter1.Sort(TElement[] elements, Int32 count) at System.Linq.OrderedEnumerable1.<GetEnumerator>d__0.MoveNext() at System.Collections.Generic.List1..ctor(IEnumerable1 collection) at System.Linq.Enumerable.ToList[TSource](IEnumerable1 source) at DSCM.DSCM.updateRecentNodes() at DSCM.DSCM.refMpData_Tick() at DSCM.DSCM._Lambda$__2(Object a0, EventArgs a1) at System.Windows.Forms.Timer.OnTick(EventArgs e) at System.Windows.Forms.Timer.TimerNativeWindow.WndProc(Message& m) at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

I won't be able to be at a PC with Dark Souls running for 9 hours or so, so hopefully you can see what's causing it.

Chronial commented 8 years ago

yes, #8 was the cause. The exception is triggered in this line: https://github.com/Wulf2k/DaS-PC-MPChan/blob/f6c5ca565b880/DaS-PC-MPChan/DSCM.vb#L674. The sort causes a compare between all the values returned by the lambda function.

New values are added to that column as Long, but the values in the registry were loaded as Integer. Apparently these are incomparable types and Int32.CompareTo(Int64) then raises an ArgumentException.

I have little experience with VB.Net and had assumed that ValueType would prevent this – apparently I was wrong :).

Wulf2k commented 8 years ago

I was just about to release a version breaking the sort until I can get home and do a better job.

Should I go with that, or do you have a better fix?

I'm stepping into a meeting at work in 30 minutes, so after that I'll most likely be unable to accept any changes until later tonight.

The ones in the registry in the old format should be converted to long when they're loaded. They're saved in the registry as a string, so it really shouldn't care what they were originally saved as.

Chronial commented 8 years ago

Now that #8 is merged, the bug should be gone. The problem wasn't how they were stored into the registry – before #8 they were loaded as Integer.

Wulf2k commented 8 years ago

There are current reports from the latest build of it complaining of Int32.

Chronial commented 8 years ago

Ah, the latest release included #8? I though it wasn't based on the on the order of your commits. If it is and the error is still surfacing in that version, make a release without the OrderBy line. But I can't see how that could happen.

Chronial commented 8 years ago

A manual cast to Long could also work:

recentNodes = recentNodes.OrderBy(Function(row) CType(row.Cells("orderId").Value, Long)).ToList()

Wulf2k commented 8 years ago

I'll give that a shot.

Does it seem to run fine on your end?

I really shouldn't be doing untested code releases while I'm at work, but I'll start implementing some Best Practices later =]

Chronial commented 8 years ago

It ran fine on my end with #8. It also works with the cast. But that is obviously not tested extensively :)

Chronial commented 8 years ago

For the releases, I think having beta and stable would make sense. This way changes are tested by the playerbase, but people who just want dscm to work have that option, too.

Wulf2k commented 8 years ago

Yeah, I was just thinking along those lines. DSCM's getting popular enough, it's time to handle things a bit more professionally.

I bungled this release a bit and probably annoyed some people but no real harm done. Good eye opener though.

Wulf2k commented 8 years ago

If you ever need to discuss anything with me outside of Github feel free to email at wulf2k@gmail.com