Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some helper methods to the Item Scanner, Movement, Journal and Tiles #18

Closed
wants to merge 2 commits into from

Conversation

DarkLotus
Copy link
Contributor

@DarkLotus DarkLotus commented Jul 28, 2018

As per title, just some minor additions to make life easier.

@Crome696
Copy link
Collaborator

Thank you for your Request! As we speak I´m moving to another place and dont have any computer to review your code. I will do this asap I´m able to do so (prolly mid\end August).

@DarkLotus
Copy link
Contributor Author

DarkLotus commented Jul 30, 2018 via email

@Crome696
Copy link
Collaborator

Well due close to zero interest i kinda stopped the further development for a while now. I stopped playing uo like a year ago and planned only to develop sdk further. But for some reasons i kinda miss uo. So planning on coming back myself after moving and then i will also update SDK further. Any support and help is appreciated.

@DarkLotus
Copy link
Contributor Author

DarkLotus commented Jul 30, 2018 via email

@Crome696
Copy link
Collaborator

So i reviewed your commited Changes and request a few changes:

  • ScriptSDK.sln you added your locale configurations into it including your own project. Please remove that.

  • ScriptSDK/Attributes/JournalHelper.cs I saw you added a function public bool InJournal(string[] content). The idea is good but then we need also an IEnumerable method for this and that has to call a logic that checks in journal same way then with Arrays. As second comment I will take a look if it wouldnt be faster to get a copy of whole Journal by sdk and then iterate through. The process of SDK is -> Method sends request to stealth, Stealth replies and send a result. Those few bytes stacked on a big journal will make it slow. C# will be faster to iterate through a big list then Stealth by handle the packets all time. It also avoids any overflow by Stealth. Not your Issues but just a few remarks. (You can see this when using clilochelper. Stealth is far slower with cliloc checks as if you read them yourself and just iterate through that. So as result : If you find the result fast \early then your way of iterate through is better. For full scans a completed Journallist call and iterate through that will be better.

  • ScriptSDK/Attributes/MovingHelper.cs Here we should overload the functions which previously called byte and now sends a Map struct. The reason is, its nice to use an enumeration and then "cast" it to byte but freeshards might have own maps and could not use those functions without an extending of SDK. So please correct that again, make the original function as byte and add a second function ( its called overloading) with Map as Parameters which calls the original function and pass Map as byte.

ScriptSDK/Stealth API/Stealth.cs Same as Movinghelper. We should avoid doublecode. Stealth.cs is the raw API and should mirror exact what Stealth expose. so please remove that change there and call it by the original movinghelper functions.

Thank you for your submissions and i hope we will see more code by you

Cheers Crome

@DarkLotus DarkLotus closed this Dec 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants