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

Have sensor mods use events for updates? #70

Open
originalfoo opened this issue Aug 17, 2016 · 16 comments
Open

Have sensor mods use events for updates? #70

originalfoo opened this issue Aug 17, 2016 · 16 comments
Milestone

Comments

@originalfoo
Copy link

I've added a bunch of new sensors to my mod and amongst other things it seems the remote.call api is somewhat laggy. Would it be better to allow sensor updates to be sent via events?

Mockup of code in a sensor mod:

local evoEvents = remote.call( 'EvoGUI', 'events' )
local sensor_update = evoEvents.update

local function updateMySensors()

  local someVals = something()

  game.raise_event( sensor_update,
    { mySensorID, val1, val2, ... }
  )

end

On receiving end (in EvoGUI) the handler does something like:

local function updateHandler( update )
  local sensor = update[1]
  local display = localeCache[sensor]
  for i = 2, #update do
   display[i] = update[i]
  end
  -- then set caption of label for sensor to display
end

Obviously very crap code in samples above, but hopefully enough to show general idea. Instead of sending table it might be easier to just pass multiple params to the raise_event(), first being sensor id, second being locale string, any others being params to locale?

If my understanding of how events work is correct, it will also allow the game to hold the event until a later frame in order to maintain UPS and FPS.

@narc0tiq
Copy link
Owner

I don't understand, laggy in what way? All that happens when you call remote.call("EvoGUI", "update_remote_sensor", [whatever]) is that it changes what's stored in the sensor table under the "line" keyword, so it should only take noticeable time if the between-sandbox dispatcher is just horrendously slow.

Now, if the dispatcher does happen to be that slow, maybe we should add a new API for a multi-sensor update (and while we're at it, create and delete, as well, why not) so that you can push the data for all the sensors at once.

Events just change that from a point-to-point dispatcher to a multi-casting one, giving no real reason to believe they'd be any faster. I could see supporting them as an alternate API, I'm just not convinced it would improve on this problem without also letting them handle multiple sensors in one event.

While we're at it, I'm sure it would help if your remote sensor were a single multi-line sensor with per-sensor options for turning specific parts off/on; then there wouldn't need to be a multi-sensor update method because we'd be doing a single dispatch again...


Honestly, my main thought right now is that the remote sensors are very much an afterthought and it shows. I'd like to change their API (to support #58, #61, #68, #69), as well as force the built-in sensors to use the very same API -- ensuring that everything the built-in sensors can do, the external ones can too. But it might be a while until I can scrape together the time for all that.

In the mean time, the quickest solution to your specific problem would be to get your tile data sensor merged into core EvoGUI: you could take advantage of being internal and have much more control over your own sensor UI, like the player_locations sensor does.

@narc0tiq narc0tiq modified the milestone: 0.5 Aug 18, 2016
@originalfoo
Copy link
Author

I'd rather keep them separate and then iterate EvoGUI to bring remote sensor feature set up to par with bundled sensors.

Mind if I have a go at implementing the event and also #58 + #68 ? Should be able to get an initial implementation up by the weekend.

@originalfoo
Copy link
Author

Uhm, so... I started editing and might have got a bit carried away... I'm currently in the middle of a complete rewrite of Evo.

So far I've replaced the whole sensors API (not wired it in to main code or GUI yet)...

  • Each remote method can handle multiple sensors, is multi-player capable (including a * wildcard for applying an update to all players), and remote sensors can now define their own options, etc.
  • Ability to set default display mode is also added, along with new features like "auto hide" (if a sensor value is nil, it is automatically hidden rather than using default value). For example, you could have sensor that indicates day/night cycle that only appears at dawn/dusk, etc.
  • I'm adding additional option types - button, glyph, label, etc.
  • I've added events for most things, and am working on helper functions that mods can use to quickly update sensors based on the events.
  • There's also ability to update value and/or color for sensors, eg. so you could retain existing value and just change color = flashing sensors are go!
  • I've added a simple notifications API, where notifications are in different style and can timeout after x ticks.
  • The legacy API remains for backwards compatibility.

Unable to show any of this in action yet as I'm still mid-way through re-writing the main code and GUI. Will probably be Monday before a preview is ready.

Speaking of GUI... I've also started writing some complete new APIs:

  • Toolbar - a strip across top of screen which mods can add their buttons, etc, in a consistent aesthetically-pleasing manner, via easy to use API. As a demo mod I'm planning to add a search box allowing players to quickly find stuff on map, along with 'map bookmarks' feature to quickly teleport to stored locations.
  • Drop panels - panels that go alongside the
  • Sidebar - a strip at side of screen where mods can put more advanced UI. The API is more basic to allow maximum freedom of what goes in sidebar.
  • Extended settings - ability to turn off things on toolbar in addition to the usual sensor stuff. Planned features include ability to re-order items and, at a much later date, 'alert thresholds' (eg. to change color or play sound if sensor goes over threshold).
  • Theming - very early stages, but I'm hoping to allow Evo theme to be moddable, and player can choose their theme in the settings. If the Factorio GUI wasn't so pants, we'd be able to auto-theme based on player color etc. Hrm, that reminds me to add a demo tool bar button for changing player color...

So yeah, got a bit carried away and am now drowning in refactor for so many hours that I have lost the ability to blink.

@narc0tiq
Copy link
Owner

I started editing and might have got a bit carried away...

Yep, I'm familiar with that feeling! I'm glad you've made such excellent progress and look forward to seeing your code (and seeing it in action, as well). Really, all of that sounds wonderful, great job!

@originalfoo
Copy link
Author

Here's what I'm trying to achieve (mockup):

evo

The GUI API is driving me nuts, especially trying to keep track of different GUI state for different users. I'm having a huge mental block trying to work out how to manage the state info for each player.

@originalfoo
Copy link
Author

originalfoo commented Aug 19, 2016

Here's something else I've been working on whilst taking break from the GUI stuff... An API for defining sensors in mods:

-- Import the Evo API
-- You only need this once, at the top of your control.lua script
local Evo = require '__EvoGUI__/api'

-- Define a sensor
-- You can define multiple sensors, just give each one a unique id
local mySensor = Evo.Sensor {
  -- id of your sensor, must be globally unique
  id       = 'mySensor';
  -- displayed name of your sensor, shown on settings screen
  caption  = { 'sensor.mySensor' };
  -- optional; default value of your sensor (default: nil)
  default  = { 'sensor.mySensor_default' };
  -- optional: text color of sensor display (default: '#fff')
  color    = '#f00';
  -- optional: default visibility mode (default: 'off')
  -- players can override this via Evo settings panel
  mode     = 'both'; -- off, normal, expand, both
  -- optional: should sensor auto-hide if updated with no data? (default: true)
  autoHide = false;

  -- optional: specify sensor specific options (default: no options)
  -- if specified, players can set these options to customise your sensor.
  -- numeric keys define option order; must be sequential starting from 1
  -- table values define the type of GUI control, examples shown below...
  options = {
    [1] = { type = 'label' , id = 'opt1', caption = {'sensor.mySensor_opt1'} },
    [2] = { type = 'check' , id = 'opt2', caption = {'sensor.mySensor_opt2'} },
    [3] = { type = 'button', id = 'opt3', caption = {'sensor.mySensor_opt3'} },
    [4] = { type = 'glyph' , id = 'opt4', sprite = 'some/sprite' },
    [5] = { type = 'text'  , id = 'opt5', default = {'sensor.mySensor_opt5'} },
    -- above shows just basic properties, see documentation for more
  };

  -- options: this gets called when a player changes options
  onSetting = function( this, players )
    -- useful for updating internal state

    -- 'this' is reference to your sensor object, eg. mySensor
    -- 'players' is array of: playerId = { settings }

    -- Delete 'onSetting' if you're not using it
  end;

  -- optional: this gets called when a player enables your sensor
  onCreate = function( this, players )
    -- useful for initialising your code

    -- Delete 'onCreate' if you're not using it
  end;

  -- this gets called when Evo needs updated sensor data
  onUpdate = function( this, players )
    -- example code
    local data = {}
    for player, options in pairs( players ) do
      data[player] = this:getDataFor( player, options )
    end
    -- returned data is sent to Evo
    return data
  end;

  -- often useful to have separate function for generating data
  -- you can add as many custom functions as desired
  getDataFor = function( this, player, options )
    local data = {}
    -- your code to get the data
    -- see <link to documentation> for correct data format
    return data
  end;

  -- optional: this gets called when a player disables your sensor
  onDestroy = function( this, players )
    -- useful for clearing down local data

    -- Delete 'onDestroy' if you're not using it
  end;

  -- optional: after you've tested that your sensor is working correctly,
  -- you can disable Evo's validation checks, resulting in faster performance
  validate = false;
}

-- There are additional, built-in methods on sensor obects.
-- See <link to docs> for more information

local users = mySensor:users() -- array of playerId = options
mySensor:reset( users ) -- reset player settings
mySensor:update( users ) -- force update
mySensor:destroy() -- uninstall your sensor from Evo

@narc0tiq
Copy link
Owner

I'm having a huge mental block trying to work out how to manage the state info for each player.

Maybe you've cleared that by now, but I'd simply shove it into global.gui_state[player.index] and be done with it.

Here's something else I've been working on whilst taking break from the GUI stuff... An API for defining sensors in mods:

Oh, that's really awesome. I had no idea it was possible to import from another package, that's so easy! We could even provide alias functions for the remote calls, e.g., EvoGUI.update_sensor instead of the equivalent remote.call("EvoGUI", "update_sensor", ...).

@originalfoo
Copy link
Author

Yup, I think I've got the GUI stuff nailed (going to take a week or to for me to code it though). Once I started thinking of making everything event-driven, rather than relying on remote calls, it became much easier to reason about. And all the event stuff can be abstracted to avoid modders (and us) from having to deal with the raw Factorio event API.

I'm thinking of ditching all the remote calls in favour of events. The remote calls have to wait for a return value from the remote function, and in most cases we don't need that return value. Also, the operation of the remote call, from what I can tell, is counted towards the processing time of the calling script, making it difficult to track down performance issues (as they always look like they're from calling script) - it will be easier to track down and fix performance issues on both sides if events are used.

I still need to test importing from other mods, if it doesn't already work I think there's good chance that Factorio devs will consider implementing it. I'm currently working on following APIs:

  • HUD - heads-up display API, allowing mods to create & manage their own hud panels
  • Sensor - obviously
  • Notify - notifications, a bit like transient sensors
  • Toolbar - for building toolbars (primarily the main toolbar at top, but could be used in any UI)
  • Sidebar - simplification of putting custom UI in a sidebar
  • Options - consistent options api for sensor, notify, toolbar and sidebar, but can also be used to do mod options - it's a subset of the sensor API code above (.options, onSetting, etc)

It might also be worth us considering adding some helper functions to the Evo prototypes, which other mods could use in their prototype definitions too, maybe even going so far as providing a css-like LPEG grammar which would make prototyping almost trivial.

@originalfoo
Copy link
Author

oh, and another benefit of event-based APIs... we can at some point provide a debug tool that shows what events are being fired, with ability to inspect their data, etc.

@originalfoo
Copy link
Author

I'm now using a mix of events and remote calls with a method agnostic parameter format so it's easy to switch between the two. This is primarily due to the fact that custom events can't be triggered when scripts initialise (game is not available) and also there were a few 'tasks' that need to immediately return a value from the mod.

Question: What is the remote_rebuild method actually used for? I'm assuming if someone updates their mod, but wasn't sure.

@narc0tiq
Copy link
Owner

narc0tiq commented Aug 23, 2016

What is the remote_rebuild method actually used for?

It's for when you manage to break something during development but want to keep the world. The intent is to have a way to recover from EvoGUI error loops by forcing it to forget you and re-learn you from scratch; I don't recall this ever being useful to anyone outside of devs, if only because EvoGUI has been quite stable (and when something broke, it was usually not due to player preferences).

I'd keep it, primarily because it never hurts to have a last-ditch nuke-and-pave option, in case something really stuffs up without your noticing. On the same note, it would help if it also triggered a re-initialization of all the sensors, if that's an option. This can include triggering a "please re-register your remote sensors" event.

Edit: also, it needs a better name, like hard_reset, or hard_reset_player if it's player-bound.

@originalfoo
Copy link
Author

Should it be specific to a sensor (assuming it's devs who use it while developing their sensors) or a mod or global (everything Evo is handling)? I was thinking of calling it "reboot_evo" and maybe having filters to constrain the reboot (eg. to a particular sensor or mod)?

@originalfoo
Copy link
Author

Is the simplified format of evolution factor important to keep? If the option for extended precision is removed (ie. the sensor always shows extended precision), the sensor no longer needs to be player specific and can be updated for all applicable players in a single pass.

--[[ Definitions ]]---------------------------------------------------------

-- luacheck: globals game

local floor     , format
    = math.floor, string.format

local refreshSensor = function() return true end


--[[ Sensor ]]--------------------------------------------------------------

_G.Evo.Sensor {

  id        = 'evolution';
  mode      = 'normal';
  minTicks  = 30;

  validate  = true; -- change to false for production use

  -- removed options; display same factor precision to all players = faster

  onEnable  = refreshSensor;
  onSetting = refreshSensor;

  onUpdate  = function()
    -- Note: string.format(%.4f) isn't platform-agnostic, hence integer math

    local factor = game.evolution_factor * 100
    local whole  = floor( factor )
    local part   = floor( (factor - whole) * 10000 )

    return { ['*'] = { value = { 'sesnor.evolution.display',
      format( '%d.%04d%%', whole, part )
    }}}

  end;--onUpdate
}

@narc0tiq
Copy link
Owner

Is the simplified format of evolution factor important to keep?

I've had people wishing for both, so yes, it's important. For the most part, anything that's now an option has been requested in both forms.

@Afforess
Copy link
Collaborator

local Evo = require '__EvoGUI__/api'

that.... that's a thing? Damn, now a lot of things I've done in the past seem silly. @aubergine10 I'm to blame for the existing complicated way to add sensors.. curious about the final outcome here.

@originalfoo
Copy link
Author

@Afforess no, sadly it doesn't work currently - the devs think it works, but in reality it only currently works for core mod (as that's already in the search path) - if possible, please lend your voice to this feature request: https://forums.factorio.com/viewtopic.php?f=28&t=32989

Currently I've paused work on the Evo stuff until we find out what's happening in Factorio 0.15 - the UI rewrite, the new mod settings API, etc... Looks like lots of stuff will be changing, most of which will affect Evo.

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

No branches or pull requests

3 participants