MV - Community Lighting MV & MZ

● ARCHIVED · READ-ONLY
Started by ImaginaryVillain 1279 posts Page 3 of 64 View original ↗
  1. Eliaquim
    #41 Source
    ImaginaryVillain said:
    I guess I should learn my way around Github then. Seems interest in this is way higher than I expected, which is great! :LZSexcite:

    As for ES6 the big joke behind the original plugin was it was written by a guy who hates object oriented programming. So a lot of the work so far has been deciphering it and working it towards something that's well... Not a very large code Jenga tower. So er..., rewriting it in ES6 is totally available for anybody who wants to do it. :LZSwink:

    @Eliaquim
    Unfortunately your version gives this error...
    View attachment 152141
    Ops! My bad!
    This should be fixed now. I forgot to remove the "tiletype" argument here, on line 383:
    this[result](command, args, tiletype);
  2. nio kasgami
    #42 Source
    you welcome I might switch it to Typescript as well if you guys are comfortable with it.

    I think refactoring the code to be more similar to RM MZ standard will already make it easier for many people to manipulate it.
    Of course, typescript will look the same just with types.
  3. ImaginaryVillain
    #43 Source
    If you want to, that's the exciting part of this experiment. Seeing what people do with it. :LZSexcite:

    @Eliaquim
    It seems to work fantastically on my end, good job! I'll let some other people test it as well before adding it to the main post. :LZScheeze:
  4. Fortunastreet
    #44 Source
    Ok, I will post it this evening after work hours and after some refactoring.

    PS: Yes, using github or any other VCS would be nice to avoid stepping on each other's toes.
  5. Aesica
    #45 Source
    Eliaquim said:
    For loops
    There is a common mistake that a lot of coders do, which is to check the length property in the loop body several times. When you make a for loop:
    JavaScript:
    for (let i = 0; i < $gameMap.events().length; i++)
    Each and every time he runs, it will take a property lookup for $gameMap.events().length.
    We do not need this(unless the length of the iterable array or object can change during the loop).
    And also it will be searching for a 'variable' outside the loop scope in each loop.
    Instead of that, we can use a local variable to save time and just do only property lookup:
    JavaScript:
    for (let i = 0, len = $gameMap.events().length; i < len; i++)

    There is also another thing to speed up the loop if the order to iterate the array doesn't matter.
    Is reversing the order in which the loop runs, like this:
    JavaScript:
    for (let i = $gameMap.events().length;i--;)
    We see now that we just set the length property to the iterator variable.
    And we set the loop control condition to 'i--'.
    Control conditions are compared to the true value. And any number that is not zero, is already true.
    So now, we are making a simple comparison against 0, which means that before reversing the order we are making two comparisons inside the control condition, per iteration:
    'i < len = Is iterator value less than the total(len) and this value is true?'
    But after reverse the order, we are only making one comparison per iteration:
    'i-- = Is this value true?'

    Example in line 521:

    JavaScript:
                    for(let i = $gameMap.events().length; i--;){
                    if ($gameMap.events()[i]) {
                        for (let j = lightarray_id.length; j--;) {
                            if (lightarray_id[j] == lightid) {
                                let mapid = $gameMap.mapId();
                                let eventid = $gameMap.events()[i]._eventId;
                                let key = [mapid, eventid, 'D'];
                                $gameSelfSwitches.setValue(key, false);
                            }
                        }
                    }
                }
    This is...only half true, at least for the chromium-based engine RMMV runs on. I ran some tests using the various methods using this quick throw-together in the RMMV debug console:

    Code:
    function forTest()
{
    var test = new Array(90000000).fill(undefined);
    var timeStamp = new Date().getTime();
    var result;
    for (let i = 0; i < test.length; i++) test[i] = 1;
    result = "test.length & i++: " + (new Date().getTime() - timeStamp);
    timeStamp = new Date().getTime();
    var len = test.length;
    for (let i = 0; i < len; i++) test[i] = 2;
    result += "\nlet len = test.length:  " + ((new Date().getTime()) - timeStamp);
    timeStamp = new Date().getTime();
    for (let i = test.length; i--;) test[i] = 3;
    result += "\ni--:  " + ((new Date().getTime()) - timeStamp);
    return result;
}
forTest();
    (Note: the "let len = ..." part actually declares len using var, but that's pretty inconsequential for the purposes of this test)

    And here were the results:
    Oa9pk9D.png

    So while it's true that accessing the length property of an array for every iteration cycle is slower than assigning it to a variable once, then accessing that variable, the backward iteration actually performs slightly worse so I wouldn't use it.

    nio kasgami said:
    you welcome I might switch it to Typescript as well if you guys are comfortable with it.
    Does MZ use typescript? I haven't been paying as much attention to it compared to most others I imagine. That said, I don't know a whole lot about typescript and how it would integrate with RMMV's javascript, so if being in typescript requires extra work in order to add it to a project, I'd hold off on it.
  6. Eliaquim
    #46 Source
    So while it's true that accessing the length property of an array for every iteration cycle is slower than assigning it to a variable once, then accessing that variable, the backward iteration actually performs slightly worse so I wouldn't use it.
    Hi!
    Thanks for the test!
    Wow, I will try to do another test here too. I remember when I learned that, in my tests, it was faster. Maybe I will do other tests here on my plugins to see if it really matters.
    I found that tip on this book High Performance Javascript, but it is from 2010. Maybe something changed in the browsers that make this irrelevant (or even worse, like in your tests).
    But for a loop with this much of iterations(90000000) I'm not sure if this example will apply either way since it approaches with another method for optimizing large arrays.
    But thanks for the heads up! :)
  7. Fortunastreet
    #47 Source
    I didn't finish my refactoring, but I found what looks like an error who seems to exist
    even in the original script.

    Line 522 and 2149:
    JavaScript:
    let key = [mapid, eventid, 'D'];
$gameSelfSwitches.setValue(key, false);

    The self-switch being modified should be the one refereed in the Kill Switch parameter, right?

    @Aesica

    Other remark about Community.Lighting.ReloadMapEvents (line 1710):

    JavaScript:
            for (let i = 0; i < event_eventcount; i++) {
            if ($gameMap.events()[i]) {
                if ($gameMap.events()[i].event()) {
                    let note = getTag.call($gameMap.events()[i].event());

                    let note_args = note.split(" ");
                    let note_command = note_args.shift().toLowerCase();

                    if (note_command == "light" || note_command == "fire" || note_command == "flashlight") {

                        event_note.push(note);
                        event_id.push($gameMap.events()[i]._eventId);
                        event_x.push($gameMap.events()[i]._realX);
                        event_y.push($gameMap.events()[i]._realY);
                        event_dir.push($gameMap.events()[i]._direction);
                        event_moving.push($gameMap.events()[i]._moveType || $gameMap.events()[i]._moveRouteForcing);
                        event_stacknumber.push(i);

                    }
                    else if (note_command == "daynight") daynightset = true;
                }
            }
        }
        // *********************************** DAY NIGHT Setting **************************
        daynightset = false;

    The check for an event with "daynight" in its note seems totally useless,
    as the local variable daynightset is reset just after the for loop.
    Most likely you wanted to include "daynight" as a valid light event like "light", "fire" or "flashlight"?
  8. nio kasgami
    #48 Source
    Aesica said:
    This is...only half true, at least for the chromium-based engine RMMV runs on. I ran some tests using the various methods using this quick throw-together in the RMMV debug console:

    Code:
    function forTest()
{
    var test = new Array(90000000).fill(undefined);
    var timeStamp = new Date().getTime();
    var result;
    for (let i = 0; i < test.length; i++) test[i] = 1;
    result = "test.length & i++: " + (new Date().getTime() - timeStamp);
    timeStamp = new Date().getTime();
    var len = test.length;
    for (let i = 0; i < len; i++) test[i] = 2;
    result += "\nlet len = test.length:  " + ((new Date().getTime()) - timeStamp);
    timeStamp = new Date().getTime();
    for (let i = test.length; i--;) test[i] = 3;
    result += "\ni--:  " + ((new Date().getTime()) - timeStamp);
    return result;
}
forTest();
    (Note: the "let len = ..." part actually declares len using var, but that's pretty inconsequential for the purposes of this test)

    And here were the results:
    Oa9pk9D.png

    So while it's true that accessing the length property of an array for every iteration cycle is slower than assigning it to a variable once, then accessing that variable, the backward iteration actually performs slightly worse so I wouldn't use it.


    Does MZ use typescript? I haven't been paying as much attention to it compared to most others I imagine. That said, I don't know a whole lot about typescript and how it would integrate with RMMV's javascript, so if being in typescript requires extra work in order to add it to a project, I'd hold off on it.
    not it doesn't I am the one that will be making the definition files for MZ once it get out. As I am working with Kino on allowing other superset language support for plugin making.

    as for extra works? it's just javascript with Static typing by itself. you compile the same than when you Bundle your plugins.

    File splitting should be something that every programmer who do big plugin project should do.
    Keeping everything in one big file is bad practice when doing the devlopment.
  9. Aesica
    #49 Source
    Eliaquim said:
    Hi!
    Thanks for the test!
    Wow, I will try to do another test here too. I remember when I learned that, in my tests, it was faster. Maybe I will do other tests here on my plugins to see if it really matters.
    I found that tip on this book High Performance Javascript, but it is from 2010. Maybe something changed in the browsers that make this irrelevant (or even worse, like in your tests).
    But for a loop with this much of iterations(90000000) I'm not sure if this example will apply either way since it approaches with another method for optimizing large arrays.
    But thanks for the heads up! :)
    The number of elements in the array can be changed to pretty much whatever you want to test with. I just chose a large number to allow for greater precision in the results. (A smaller number might not show any difference between iterating in either direction. Also yeah, JS has changed a lot since 2010 and a lot of browsers have varying optimizations. For example, cloning an array via the spread operator is lightning fast in Chrome, but slice is faster in Firefox.

    Alexandre said:
    I didn't finish my refactoring, but I found what looks like an error who seems to exist
    even in the original script.

    Line 522 and 2149:
    JavaScript:
    let key = [mapid, eventid, 'D'];
$gameSelfSwitches.setValue(key, false);

    The self-switch being modified should be the one refereed in the Kill Switch parameter, right?
    Honestly, no idea, but I suspect that's the case. That's original Terrax code. :)

    Alexandre said:
    Other remark about Community.Lighting.ReloadMapEvents (line 1710):

    JavaScript:
            for (let i = 0; i < event_eventcount; i++) {
            if ($gameMap.events()[i]) {
                if ($gameMap.events()[i].event()) {
                    let note = getTag.call($gameMap.events()[i].event());

                    let note_args = note.split(" ");
                    let note_command = note_args.shift().toLowerCase();

                    if (note_command == "light" || note_command == "fire" || note_command == "flashlight") {

                        event_note.push(note);
                        event_id.push($gameMap.events()[i]._eventId);
                        event_x.push($gameMap.events()[i]._realX);
                        event_y.push($gameMap.events()[i]._realY);
                        event_dir.push($gameMap.events()[i]._direction);
                        event_moving.push($gameMap.events()[i]._moveType || $gameMap.events()[i]._moveRouteForcing);
                        event_stacknumber.push(i);

                    }
                    else if (note_command == "daynight") daynightset = true;
                }
            }
        }
        // *********************************** DAY NIGHT Setting **************************
        daynightset = false;

    The check for an event with "daynight" in its note seems totally useless,
    as the local variable daynightset is reset just after the for loop.
    Most likely you wanted to include "daynight" as a valid light event like "light", "fire" or "flashlight"?
    That was checked in the original code and was part of that big long fire || light || ...etc if block, but I split it out since it had another conditional check for "daynight" inside the block. I assume Terrax intended for daynight to be usable on events or maps alike originally, but then never got around to properly implementing it for events. Yeah, that else if... line can safely be deleted.

    nio kasgami said:
    not it doesn't I am the one that will be making the definition files for MZ once it get out. As I am working with Kino on allowing other superset language support for plugin making.

    as for extra works? it's just javascript with Static typing by itself. you compile the same than when you Bundle your plugins.

    File splitting should be something that every programmer who do big plugin project should do.
    Keeping everything in one big file is bad practice when doing the devlopment.
    I guess what I'm saying is I don't want other developers to feel like they have to download and and install a bunch of other stuff just to make this work with their projects, so unless typescript is supported natively by MV (I don't think it is) or MZ when it finally comes out, I'm going to have to be the unfortunate "no" vote for this.
  10. ImaginaryVillain
    #50 Source
    I admit I know nothing about typescript, though the goal of the plugin is to be as fast and efficient as possible, preferably with the least effort on the end user. So if it needs something extra outside of the plugin to work, I'm pretty against that as well. Expecting an end user to do anything beyond the most minor of things just invites a lot of unnecessary support questions, and poor plugin adoption levels. Plus it sounds like people working on the plugin would have to learn something entirely new just to do so, which would just slow development.

    On a different note, has anybody else tested @Eliaquim 's version? It seemed to be faster for me, but I admit I don't test the tile/region/daynight stuff (thought I should probably make a project for just that). So I didn't want to add it to the first post till someone has.
  11. Aesica
    #51 Source
    ImaginaryVillain said:
    On a different note, has anybody else tested @Eliaquim 's version? It seemed to be faster for me, but I admit I don't test the tile/region/daynight stuff (thought I should probably make a project for just that). So I didn't want to add it to the first post till someone has.
    I tested it a few moments ago, and so far so good. I do have the following thoughts though:
    1. I'm unsure putting most of that stuff on Game_Interpreter's prototype is really necessary unless you need access to its methods specifically, and it doesn't seem like anything of the newly-added functions do at a glance. I say this because it opens the door for other plugins to trip over each other. Like, if another plugin also has Game_Interpreter.prototype.tint, there's gonna be problems. Instead, I'd attach those things to Community.Lighting instead (which can be abbreviated as $$ inside the main anonymous function) to avoid conflicts.
    2. It seems I'm not the only one that often forgets to update all the version numbers in comments. Both of the numbers in comments are still 1.017
    Other than that, everything seems to work great. :D
  12. Fortunastreet
    #52 Source
    Here is as promised a version with lighting during battle.
  13. nio kasgami
    #53 Source
    I think you guys misread a lot of what I am saying. but whatever, we should still be able to split files because working on a 5k file every time is exhausting.
  14. Aesica
    #54 Source
    Alexandre said:
    Here is as promised a version with lighting during battle.
    That's pretty cool, but if I may nitpick a bit, the tint layer should probably be drawn over the battlebacks layer and not the entire battlefield. The default night tint looks like this in battle, which kind of washes out the battlers and especially animations, especially those that should be a bit brighter and glowy:

    ZH0CiSQ.png

    Also, it looks like you did what I did earlier by working on an earlier version than the latest one. In this case, this versoin is before @Eliaquim optimized the for loops.

    Other than that, it's awesome. I've been meaning to look into some way to do this myself, actually. :)

    nio kasgami said:
    I think you guys misread a lot of what I am saying. but whatever, we should still be able to split files because working on a 5k file every time is exhausting.

    Oh don't misunderstand. If you want to make a typescript fork of it when all the changes have settled down, by all means do. You're not being excluded by any means.
  15. spec3
    #55 Source
    I could not read all the replies yet, but seeing this is so exciting! Good job guys!
  16. ImaginaryVillain
    #56 Source
    So look at this and seeing how we are producing new versions faster than people are testing this. I propose we put the most feature rich bug free version in the first post as the main version. Then we create a section for variant versions. This way when we have wildly different variants there is no push to merge them together, and we have more time to examine them. Then anybody who wants to see how the code is done, and potentially improve on variants or the main script can get access to them, study them and potentially get ideas, fork stuff, etc.

    Why go this route? Well each of us seems to have different goals. I for instance believe the script is bloated so the version I use is focused on trimming it down. Currently it's 1,396 lines, with the main function being 89 lines. Obviously by removing close to 1,300 lines of code it's far faster. But I also yanked out a lot of features I don't use because it's specifically for my game.

    Obviously that won't work for everybody, but if I discover something I think it worth doing to the main script, I'll offer it up. So basically a collaboration on making the fastest, most compatible, feature rich main version. Then a loose collaboration on the rest of the stuff. :LZScheeze:

    @Alexandre Nice! I admit to only ever running a battle in MV 3 times.... So I will let others test that. I'm a map guy myself. :LZSwink:

    @nio kasgami
    Honestly, I'm an artist/hobbyist programmer just here to live a dream of making a game. I'm got no classical training as a programmer, I learned everything I know by studying and manipulating scripts. (Part of why I'm so good at hacking them up to make them more efficient). So I'm more than happy to learn about stuff, and would love a better explanation. :LZSexcite:

    Also if we go the route of having variant scripts, you could just have a version up as well. In fact I'm not really against you making a version that's typescript. Just realize, it might not get adopted by the group since we don't know much about it. Also if you wish to make a split up version, that's an option too. Of course this is all committee based so if people end up liking that version better, they'll just start tweaking with it over other version. :LZSwink:
  17. nio kasgami
    #57 Source
    it's fine lol I am an artist as well I am not an trainee in programming far from it I just do programming as an hobby for a long time.

    for the typescript it's fine I don't care much of it.
    although making the code as MZ standard should be prior.
  18. Fortunastreet
    #58 Source
    Aesica said:
    That's pretty cool, but if I may nitpick a bit, the tint layer should probably be drawn over the battlebacks layer and not the entire battlefield. The default night tint looks like this in battle, which kind of washes out the battlers and especially animations, especially those that should be a bit brighter and glowy:

    Actually I liked this effect, but it is clearly a subjective preference. It is typically something who can be a plugin parameter.

    Aesica said:
    Also, it looks like you did what I did earlier by working on an earlier version than the latest one. In this case, this versoin is before @Eliaquim optimized the for loops.

    Other than that, it's awesome. I've been meaning to look into some way to do this myself, actually. :)

    Yep, I missed that part, I will add those changes asap.

    ImaginaryVillain said:
    @Alexandre Nice! I admit to only ever running a battle in MV 3 times.... So I will let others test that. I'm a map guy myself. :LZSwink:

    As an original boss fight lover, I always found strange that the plugin was limited to the map.

    nio kasgami said:
    it's fine lol I am an artist as well I am not an trainee in programming far from it I just do programming as an hobby for a long time.

    for the typescript it's fine I don't care much of it.
    although making the code as MZ standard should be prior.

    I agree that making it compatible with MZ standards while it comes out is a greater priority.

    I will probably drop a V2 this evening with:
    -The position of the battle mask layer as parameter.
    -Eliaquim's work
    -The killswitch bug I mentioned before corrected.
    -The killswitch parameter as a select in the plugin manager (may be I will edit some others parameters in the process)
  19. Fortunastreet
    #59 Source
    @Aesica

    Capture.PNG

    Here is how it looks like when placing the light mask between the background and the battlers.
    Do you think that this arrangement would be a good alternative to the default one?
  20. Aesica
    #60 Source
    Alexandre said:
    @Aesica

    View attachment 152413

    Here is how it looks like when placing the light mask between the background and the battlers.
    Do you think that this arrangement would be a good alternative to the default one?
    Yeah, that's perfect! And I agree, a plugin parameter is probably the best way to handle that so people can choose either method to suit their tastes. Nicely done!