search

multitheftauto / mtasa-resources

This project maintains a list of up-to-date resources that come with Multi Theft Auto.
https://multitheftauto.com
MIT License
152 stars 152 forks source link

internetradio: refactor #507

Closed ds1-e closed 2 months ago

ds1-e commented 3 months ago
ds1-e commented 3 months ago

Ready for testing

Fernando-A-Rocha commented 3 months ago

I noticed that all config in this resource is done in Lua. Due to the amount of modifiable variables and their complexity (table of radio stations, rgb colors, etc), I'm okay with keeping them as is, and not making meta.xml settings for this resource. Unless anybody thinks it would be easy to put all this as xml settings...

T-MaxWiese-T commented 3 months ago

I noticed that all config in this resource is done in Lua. Due to the amount of modifiable variables and their complexity (table of radio stations, rgb colors, etc), I'm okay with keeping them as is, and not making meta.xml settings for this resource. Unless anybody thinks it would be easy to put all this as xml settings...

I think it would make sense for most variables. And with the table you could put each radio station in a group, which means you could also enter your own radio name and URL. There is also the possibility to work with tables in the settings, but this is not very attractive due to the small width of the text field for changes and the display. In admin2 you could consider making the text field larger so that you would have a much better overview and you would have to simplify the display and thus translate it. But the bottom line is that I find this rather impractical. It's still easiest if you have individual variables that you can change.

Fernando-A-Rocha commented 3 months ago

You say that it's viable to have all settings in Meta.xml, I'm interested in seeing how that can be implemented 😅 what do you think? @srslyyyy

ds1-e commented 3 months ago

I believe it is fine as it is now (but i do think of adding ability to change key for player itself). Feel free to change it, once it gets merged.

Fernando-A-Rocha commented 3 months ago

I agree tbh, it is unclear how to make this resource more easily configurable for now. Lua vars is fine. Feel free to merge, imo.

jlillis commented 3 months ago

I don't think we need all the possible configuration options available in via admin settings. The only one I can think of would be nice is the setting that allows/disallows players to place speaker objects.

Everything looks good to me but I'd like @Dutchman101 to take a look as it was his resource originally.