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

Change checkParameters() error messages to always give name of function #3

Merged
merged 1 commit into from
Sep 5, 2019

Conversation

Merudo
Copy link
Member

@Merudo Merudo commented Sep 1, 2019

  • Change wrong number of parameters error messages to always display name of function
  • checkParameters method in AbstractFunction changed to take functionName as first argument
  • Need to do: change every checkParameters(parameters) in Maptool project to checkParameters(functionName, parameters)
  • Parser changes to close Change parser to provide more helpful error messages for incorrect # of parameters #2
  • New error message for math functions. For example, for Floor(),

Old error message:

Invalid number of parameters 0, expected exactly 1 parameter(s)

New error message:

Function 'Floor' requires exactly 1 parameters; 0 were provided.


This change is Reviewable

Merudo added a commit to Merudo/maptool that referenced this pull request Sep 1, 2019
…me of function

- Change wrong number of parameters error messages to always display name of function
- Needs RPTools/parser#3 to work
- Close RPTools#629
Merudo added a commit to Merudo/maptool that referenced this pull request Sep 1, 2019
- Change wrong number of parameters error messages to always display name of function
- Needs RPTools/parser#3 to work
- Close RPTools#629
- Change wrong number of parameters error messages to always display name of function
- checkParameters method in AbstractFunction changed to take functionName as first argument
- Need to do: change every checkParameters(parameters) in Maptool project to checkParameters(functionName, parameters)
Merudo added a commit to Merudo/maptool that referenced this pull request Sep 1, 2019
- Change wrong number of parameters error messages to always display name of function
- Needs RPTools/parser#3 to work
- Close RPTools#629
Copy link
Member

@cwisniew cwisniew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 13 of 13 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@cwisniew
Copy link
Member

cwisniew commented Sep 4, 2019

need to update so that the error messages are internationalized. I realise its another painful dependency but its equally if not more so painful for anyone developing macros with MapTool in a different language if they get an error they can't understand.

I am open to other options if this is too painful (such as creating a utility class for macros in MapTool and changing functions to call that, I am pretty sure if we made such a utility class we could find other common macro code that could be moved to it as well).

Merudo added a commit to Merudo/maptool that referenced this pull request Sep 5, 2019
- Change wrong number of parameters error messages to always display name of function
- Needs RPTools/parser#3 to work
- Close RPTools#629
Copy link
Member

@cwisniew cwisniew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@cwisniew
Copy link
Member

cwisniew commented Sep 5, 2019

Current error message are also not internationalised so I will approve and open another issue for internationalisation of this project.

@cwisniew cwisniew merged commit 1ace76f into RPTools:master Sep 5, 2019
Merudo added a commit to Merudo/maptool that referenced this pull request Sep 6, 2019
- Change wrong number of parameters error messages to always display name of function
- Builds on RPTools/parser#3 to work
- Close RPTools#629
JamzTheMan added a commit to RPTools/maptool that referenced this pull request Oct 4, 2019
* Fix token ids reset on server start

- Fix so that token ids stay the same after server start
- Add javadocs info for Zone & Campaign constructors
- Close #619

* Fix cut & paste changes token id

- Fix so that Cut & paste doesn't change token id
- Subsequent pastes will result in different token ids
- Close #624

* Fix "paste" option remaining grayed out after Copy/Cut token

- Fix "Paste" menu not getting updated after performing the Copy/Cut action from popup menu
- Close #621

* Fix switching from/to the measure tool not updating cursor right away

- Change how the measurer tool cursor updates, now based on attach/detach instead on paintOverlay
- Fix cursor not updating right away when switching from measurer to pointer
- Fix cursor not updating right away when switching to measurer from label, fog or VBL tool
- Close #601

* Fix bug created by dragging token from library to map

- Fix token having duplicated ids if a cut was followed by dragging token to map then a paste (induced by #625)
- Fix dragging token from library to map not ungraying the "Paste" option (fully completes #630)

* Fix incorrect message for assert() when the second parameter is number

- New message: Second argument to "assert": "{second parameter}" must be of type String
- Close #637

* Translate a few UI elements in french

* Consolidate utility methods for macro functions in FunctionUtil

- New class FunctionUtil to handle checks and type conversions for macro functions
- New internal methods checkNumberParam, getTokenFromParam, paramAsBigDecimal, paramAsBoolean, paramAsInteger, paramAsDouble, paramAsFloat, paramAsJson, paramAsJsonObject and paramAsJsonArray
- Change TokenLightFunctions hasLightSource, clearLights, setLight, and getLights to use these new methods
- Methods tested: checkNumberParam, getTokenFromParam, and paramAsBigDecimal
- First step to solve #613

* Add parameter in FunctionUtil to allow/disallow text to BigDecimal

- Add parameter allowString: if set to false, raises exception when parameter is a string that could be converted to number( ex: "0", "5", etc).

* Change ZoomFunction to use FunctionUtil

- Functions getZoom, setZoom, getViewArea, setViewArea, and getViewCenter now use FunctionUtil
- Fix functions getZoom and setViewArea not displaying errors when given too many parameters
- Fix javadocs errors
- Remove extra delimiter at the end of getViewCenter

* Add translation support for "Select Map" button

- Select Map button (top right of screen) is now properly translated
- Tested for English and French

* Change wrong nb of parameters error messages to display function name

- Change wrong number of parameters error messages to always display name of function
- Builds on RPTools/parser#3 to work
- Close #629

* Change parser implementation to 1.5.4

* Add json.path.read function (based on jayway)

- Function takes a "json" and a "path" argument
- Path should be defined according to https://github.com/json-path/JsonPath
- Support inline predicates
- Errors are printed to chat but not according to our standards
- Progress on #612

* Add error handling for json.path.read

* Add json.path.add, json.path.add/put/set, and json.path.delete functions

- Functions can modify a nested json array
- Path should be defined according to https://github.com/json-path/JsonPath
- Closes #612
- Details:

1. json.path.add(json, path, value) adds an element to a jsonArray at the path
2. json.path.set(json, path, value) sets an element in a jsonArray or jsonObject at the path
3. json.path.put(json, path, key, value) adds or sets an element in a jsonObject at the path
4. json.path.delete(json, path) deletes an element in a jsonArray or jsonObject at the path

* Consolidate methods checkNumberOfParameters and getTokenFromParam

- Replace checkNumberOfParameters by FunctionUtil.checkNumberParam
- Replace getTokenFromParam by FunctionUtil.getTokenFromParam
- Fix #613

* Fix bring to front/send to back need to be done twice

- Add ZOrder sorting of the tokens on the server at the end of bringTokensToFront / sendTokensToBack

* Update build.gradle

compile is deprecated and replaced with implementation

* Fix for ConcurrentModificationException

 * Make partialPaths thread safe and Close #328

Signed-off-by: JamzTheMan <[email protected]>

* Spotless applied

* Add new Server Option for GM to Reveal FoW for Unowned Tokens

 * Closes #665 and Closes #663
 * Added abeille-maptool-forms.jfpr project file to buil-resources

* Add playStream, stopStream, and editStream functions to stream audio

- Add playStream(uri, cycleCount, volume) to play audio from url or local file.
- cycleCount: number of times to play the resource. -1 for infinite loop. Default: 1.
- volume: volume value from 0-1. Default: 1.
- Add stopStream(uri, remove) to stop a stream. If remove is set to 1, unload the stream from memory. Default: 1. If no uri specified, stop all sounds.
- Add editStream(uri, cycleCount, volume) to change the cycleCount or volume of stream.
- Add AppPreference playStreams to enable/disable playing streams
- Ex:

[playStream("https://www.fesliyanstudios.com/musicfiles/2019-05-01_-_Undercover_Spy_Agent_-_David_Fesliyan.mp3", -1, 0.8)]

* Fix disabling streams, and make stream functions thread-safe

- Fix access HashMap to be thread safe (now accessed solely from the JavaFX app single thread)
- Fix so that disabling streams in settings also stops all streams

* Add features to playStream functions

- Move streaming methods to class MediaPlayerAdapter, as discussed in #668
- Add new parameters: startTime and stopTime to playStream & editStream
- playStream with cycleCount 0 preloads the stream but does not play it
- Add function getStreamProperties as suggested in #667
- Add support for volume slider through setGlobalVolume, but the slider needs to be created

* Fix getStreamProperties wrongly returning "PLAYING" , change ms to secs

- Fix getStreamProperties to return status "STOPPED" after song finished playing
- Change parameters startTime and stopTime to take seconds instead of ms

* Add Volume Slider and Mute Button to Toolbar

- Icons provided by @JamzTheMan

Co-Authored-By: Jamz <[email protected]>

* Add "fadeout" parameter to stopStream

- Add optional fadeout (in seconds) before closing a stream
- 0: no fadeout (default)
- Stream will stop once the fadeout period is over
- Suggested by @dorpond in #615

* Add DebounceExecutor, update ZoneRenderer.java to use debounced repaint (#611)

dispatcher.

* Add sound icons to the github directory

- Add back,forward,next,pause,play,previous,record,record_on,repeat,shuffle,shuffle_on, and stop icons
- Move mute & volume icons to correct directory
- Icons will be used for #615
- Icons provided by @JamzTheMan

Co-Authored-By: Jamz <[email protected]>

* Add support for space in uri for stream functions

- Add conversion of string to uri, replacing space into %20
- Add "FILE/:" at the start of uri if it is missing
- Fix issues raised in #667

* Fix PRs automatically failed

- Change to using jitpack for clientserver
- Fix #679

* Fix bug: "*" stop working as path in stopStream, getStreamProperties (#682)

- Add special case so that "*" isn't modified
- Fix regression created by #678
- Fix #681

* Add propertyType option for getTokens(), improve getTokens() (#677)

- Add new option "propertyType" to conditions in getTokens()
- Value can be a single type as string, or an array of types
- Simplify getTokens() code by removing the second loop. Now include/exclude is a filter property ("match")
- Improve performance of getTokens() by trimming the list continuously instead of constantly filtering the list of all tokens
- Close #676

* Json, store "null" "false" and "true" as their revelant java type internally (#672)

* jsonify "null", "true" and "false" into null true and false

* fix null handling, using the library Null object

* jsonify should be static and public

* Change getTokens to be much faster with area option (#686)

- Change so overlap is computed explicitely instead of using A*
- Much better performance
- Solves #683 for "area" but not for "range"

* ImagePanel QOL improvements (#691)

- Request only images in view from ImageManager
- Trigger repaint() from paint-affecting properties only if property
actually changes
- Calculate truncated caption heuristically
- Improve spacing
- Include caption in mouse capture area
- Antialias text
- Use interpolation when rendering scaled images

* Fix Manhattan distance, metric distance parameter with iso maps (#690)

- Fix manhattan distance while moving tokens & in distance functions
- Fix getDistance, getDistanceToXY, getTokens ignoring metric parameter for isometric maps
- Close #688

* Fix json.path functions adding extra / and \ to json objects (#693)

- Fix issue raised in #612

* Resolve #694 -- Fix asset panel scaling (#695)

* #696 Implement dragTheshold in DefaultTool

* Fix NullPointer exception when closing maptool

Fix #700

* Change getDistance to use explicit closed-form

- Improve speed dramatically while using getTokens() with a distance option
- Previous walker-based approach kept as it could be extended to take VBL & terrain into account
- Distance returned tested to be the same as previous code
- Close #683

* Fix two issues from moving token when "Snap Token While Dragging" is off

- Fix token "jumping" to a different spot once Snap-to-Grid is disabled
- Fix getDistance returning incorrect distance with NO_GRID metric
- Close #699

* Fix to getDistanceToXY and getDistance with NO_GRID metric

- Change so that getDistanceToXY and getDistance now calculate the distance from the center of the token when the token is large
- Add new "pixel" parameter to getDistanceToXY. It indicates if the x,y are for a cell or pixel (pixel: false by default)
- pixel false: getDistanceToXY gets the distance from the center of the token to the center of the specified cell
- Close #684

* Fix bug in #699 (#711)

- Fix incorrect boolean check
- Fix missing CellPoint to ZonePoint conversion

* Change volume slider to affect system sounds

- Change system sounds to be affected by volume slider & mute button
- Change system sounds to use JavaFX
- Close #709

* Fix campaign macro changes not received when connecting to server

- Fix so macro changes are now properly updated to the server
- Close #713

* Fix bug introduced by #707 with default metrics

- Fix bug where the default metric is the user's preferred one instead of the server's.
- Function affected: getDistance, getDistanceToXY, getTokens
- Fix issue mentioned in #684

* Add option "gm-self", "self", & "all" to broadcast

Close #718

* Fix getViewArea and getViewCenter returning incorrect pixels

- Change so getViewArea and getViewCenter return the actual pixels as used in setViewArea, for example.
- Close #724

Co-Authored-By: Phergus <[email protected]>

* Add parameter "players" and "delim" to execLink (#721)

- Add new parameter "players" to execLink. All players specified will have the macro run for them. Default: self.
- The parameter "players" can be a string, a list, or json array. Use new delim parameter for list or array (defaults to ",").
- execLink also accepts "gm", "gm-self", "self", "all", and "none".
- Close #716

* Add scrolling to Select Map menu (#723)

- Add new class "JScrollPopupMenu" to handle scrolling
- Class is fully functional but not complete.  Other methods relying on indexes of the components should be reimplemented before they are used
- Maximum of maps displayed at a time: 20 (scrolling will reveal them all)
- Close #356

* Add function json.toVars to turn jsonObject into variables

- keys become variable names
- values become the content of the variables
- Close #591

* Improvement to json.toVars

- Change: parameter "suffixed" is now replaced by "prefix" and "suffix", which can add a prefix and/or suffix to the names.
- Change: spaces in keys are now turned into underscore
- Change: characters not alpha-numerical other than "." and "_" are deleted from var names
- Change: function now return a JSONArray with the variable names

* Fix javadocs errors

- Fix javadocs errors; it's now possible to generate the full javaDocs
- No actual code changed
- Warnings still present, but the errors should be fixed
- Close #731

* Update ChangeLog for 1.5.5 release (#736)

First pass.

* More updates to ChangeLog plus updated credits. (#737)

* Update ChangeLog for 1.5.5 release

First pass.

* Fix ChangeLog and update credits.html

Fixed typos and missing linkage in ChangeLog.
Updated credits.html file.

Issue #735

* Changed getInfo("server") macro to use ServerPolicy.toJSON directly (#738)

Updated getInfo("server") macro function to pull JSON object from ServerPolicy instead of having duplicated code.
Updated ServerPolicy.toJSON() to include isAutoRevealMovement and isUseIndividualFOW.

* Junit tests for Json function (#741)

* jsonify "null", "true" and "false" into null true and false

* fix null handling, using the library Null object

* jsonify should be static and public

* json unitary tests

* use a resource fiel for json rather that constructed within the test code

* spotless on json tests

* Updated with Issue #670 (#743)

* Update ChangeLog for 1.5.5 release

First pass.

* Fix ChangeLog and update credits.html

Fixed typos and missing linkage in ChangeLog.
Updated credits.html file.

Issue #735

* Add missing issue 670

Updated with issue 670.

* Update parser to 1.5.5

Removed self dependency in Parser project.

* Update build.gradle to pull dicelib v1.5.5 (#749)

build.gradle updated to pull in dicelib v1.5.5
Completes issue #746.

* Closes #751

Due to how the uberJar is packaged, we need to register JAI imageIO for jpeg2000
The same fix was applied to TokenTool for the same issue.

Signed-off-by: JamzTheMan <[email protected]>

* Updates to Change Log and Credits (#754)

* Update ChangeLog for 1.5.5 release

First pass.

* Fix ChangeLog and update credits.html

Fixed typos and missing linkage in ChangeLog.
Updated credits.html file.

Issue #735

* Add missing issue 670

Updated with issue 670.

* Update ChangeLog and Credits

Updated Change Log for issues 731, 746 & 751.  Updated MapTool credits  with recent contributor and missing `</td>` tags.
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.

Change parser to provide more helpful error messages for incorrect # of parameters
2 participants