BAndysc / WoWDatabaseEditor

Integrated development environment (IDE), an editor for Smart Scripts (SAI/smart_scripts) for TrinityCore based servers. Cmangos support work in progress. Featuring a 3D view built with OpenGL and custom ECS framework
MIT License
431 stars 153 forks source link

[Feature] SAI time explanation/hints #57

Open Jinnaix opened 3 years ago

Jinnaix commented 3 years ago

Is your feature request related to a problem? Please describe.

SAI Timers are in milliseconds. WoWDE converts timers automatically into seconds and blends out previous user input and shows converted times.

Describe the solution you'd like

The user gets no information about this at first hand. Maybe a info box with an explanation that timers are milliseconds and gets converted for readability afterwards would be nice.

Additional context

image

Branel commented 3 years ago

Universally, timed events are in ms, it's just a accepted norm. One thing I can agree with this issue is that the event states 'Every X seconds' but it counts in miliseconds. Changing that might resolve the confusion.

I'll fork a PR and let me know if the changes are satisfactory.

BAndysc commented 3 years ago

The thing is, it is not that simple. There are two separate things:

The first one is directly related to how the database keeps the time - because it is in milliseconds, in the editor also puts milliseconds.

However, when it comes to how the editor shows the script later, in my opinion it is much easier to read:

Every 2.5 seconds Rather than Every 2500 milliseconds

That's why it works this way now. I don't think it is confusing, I mean - you can quickly see how it works. If you type "2" in the parameter field, the editor will say "every 0.002 seconds", so you should be able to quickly learn that you have to type 2000.

Of course it could be changed to show milliseconds always, but again, in my opinion it is easier to read seconds than milliseconds

Branel commented 3 years ago

Now this is all very subjective, I am familiar with working with milliseconds and don't mind it, that's how it is often displayed but it is a matter of preference, and how you choose to implement it. Here I think noone is wrong wrong but rather there is conflicting interest.

But if I understand your line of thought your idea is more that it should be clear that the user inputs seconds that will be converted to milliseconds? Which is indeed what the author of the report considered a solution.

BAndysc commented 3 years ago

In my opinion, it should be clear even without textual explanation, but after the talk with OP I came with an idea to add small help box that will inform about it explicitly.

Now I came with another idea - to add switch in the settings so that user could select if one wants to display time as seconds or milliseconds. While doable, I am not sure right now how much work it would require. And I do not want to spend too much time on small things