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

Various refactorings and improvements #107

Merged
merged 16 commits into from Aug 31, 2018

Conversation

Projects
None yet
3 participants
@SamuelMoriarty
Copy link
Contributor

SamuelMoriarty commented Aug 17, 2018

Just a load of random fixes/refactorings/additions I've accumulated over a few weeks.

Short summary:

  • Added some new extension funcs for rect that I thought were missing.
  • Split up Buffer.wurst into multiple files
  • Created AbstractStringBuffer as a base for custom serialization formats (groundwork for FileIO v2)
  • Improved error-handling in Persistable
  • Various fixes in stdlib that I've spotted related to max instance limit (some were hard-coded as 8191)
  • Fix bug in SyncSimple where it would get screwed over by EventListener
  • Moved around some logger functions so that log level checks get removed when using a constant log level + Log.warn/info/etc.
interface LimitedExecuteCondition
function check() returns boolean

This comment has been minimized.

@Frotty

Frotty Aug 17, 2018

Member

what's up with the whitespace?

@@ -1,6 +1,7 @@
package Printing

This comment has been minimized.

@Frotty

Frotty Aug 17, 2018

Member

Should be different PR and is still worthy of discussion

This comment has been minimized.

@SamuelMoriarty

SamuelMoriarty Aug 18, 2018

Author Contributor

moved to #108

try(() -> buffer.transfer(network.getData()))
// if there was an error during transfer, we scrap the data...
if not buffer.isValid()
boolean populateSuccess

This comment has been minimized.

@Frotty

Frotty Aug 17, 2018

Member

why are these here

This comment has been minimized.

@SamuelMoriarty

SamuelMoriarty Aug 18, 2018

Author Contributor

wdym? need to do 2 separate checks and get their result

This comment has been minimized.

@Frotty

Frotty Aug 18, 2018

Member

populateSuccess can be merged with first assignment?

4. "s10|0123456789" - string 0123456789
5. "s5|01234i10|b1b0r1.0005|" - string 012345, int 10, bool true, bool false, real 1.0005
**/
public class OrderedStringBuffer extends AbstractStringBuffer

This comment has been minimized.

@Frotty

Frotty Aug 17, 2018

Member

Should probably also be in its own file then?

This is an utility function which you can use to process arbitrarily large loops.
It functions like a while loop that will run action.run() while condition.check() == true,
however it will reset the OP limit each time after the specified amount of resetCount.
This is an utility function which you can use to process arbitrarily large loops.

This comment has been minimized.

@Frotty

Frotty Aug 17, 2018

Member

an -> a


import ErrorHandling

import public initlater HashBuffer

This comment has been minimized.

@Frotty

Frotty Aug 17, 2018

Member

can we get rid of these initlater imports?

SamuelMoriarty added some commits Aug 18, 2018

wat
@configurable public constant NW_SAFETY_CHECKS_ENABLED = true

This comment has been minimized.

@Frotty

Frotty Aug 25, 2018

Member

why rename?? if ppl config this it's breaking change.

This comment has been minimized.

@SamuelMoriarty

SamuelMoriarty Aug 26, 2018

Author Contributor

conflict with buffer's configurable of the same name
trying to import buffer in network resulted in variable name collision

@@ -16,6 +16,10 @@ interface LocalFilesCallback
In other words, this checks if the user can reliably write/read files.
This also includes a fallback mechanism for the 1.26 patch and before,
which uses the Logs\ folder to write files in.

NOTE: Since patch 1.30, the requirement for the Allow Local Files registry
setting has been lifted. This class will always succeed for future versions.

This comment has been minimized.

@Frotty

Frotty Aug 25, 2018

Member

not always probably.

This comment has been minimized.

@Kithio

Kithio Aug 25, 2018

Contributor

@Frotty What makes you assume this wouldn't always be the case?

This comment has been minimized.

@Frotty

Frotty Aug 25, 2018

Member

because the localfiles requirement has been lifted with 1.30
It will be deprecated and removed eventually.

@Frotty

Frotty approved these changes Aug 31, 2018

@Frotty Frotty merged commit 2a7219f into wurstscript:master Aug 31, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Frotty

This comment has been minimized.

Copy link
Member

Frotty commented Aug 31, 2018

🍨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment